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.

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

Inherit from fedora-bootc's tier-x on Fedora 42+ #3177

Merged
merged 14 commits into from
Nov 18, 2024

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Sep 23, 2024

There is a new tier-x in the fedora-bootc project whose goal is to
provide a common base that all variants (including tier-1) can share.

Move FCOS over to use this new tier, but only starting from Fedora 42.

This is a profound change and the start of an exciting new future! This
formalizes our relationship to other image-mode variants, encouraging us
to innovate and solve problems together in a more direct way.

Put more practically, e.g. bug fixes, new features, or temporary
workarounds that concern all/most tier-x derivatives should probably be
carried out at the tier-x level rather than the CoreOS level.

Eventually, this inheritance will be made even more explicit by having
FCOS be built FROM the tier-x image. For now, we share at the manifest
level, which is a stepping stone towards that goal.

Patches to actually dedupe our manifests with tier-x will follow. Though
note there is no change in the resulting package set here.

@jlebon
Copy link
Member Author

jlebon commented Sep 23, 2024

Targeting rawhide for now to exercise it in CI. Will retarget testing-devel once this is ready to go.

Other outstanding items:

  • verify configbot handling of git submodules
  • prepare openshift/os changes in advance

@jlebon
Copy link
Member Author

jlebon commented Sep 23, 2024

Some diff stats:

testing-devel

  • No package diff
  • No initramfs diff
  • OSTree diff:
    $ ostree diff 40.20240922.dev.0 40.20240922.dev.1
    M    /usr/etc/pki/ca-trust/extracted/java/cacerts
    M    /usr/lib/os-release
    M    /usr/lib/modules/6.10.10-200.fc40.x86_64/initramfs.img
    M    /usr/lib/sysimage/rpm-ostree-base-db/rpmdb.sqlite
    M    /usr/share/rpm/rpmdb.sqlite
    M    /usr/share/rpm-ostree/treefile.json
    A    /usr/bin/systemd-firstboot
    A    /usr/lib/.build-id/07/052139ba8f88405bf6bef78a8f51b67894a761
    A    /usr/lib/systemd/system/systemd-firstboot.service
    
    These are all expected. See related commit for the systemd-firstboot-related one. The others churn naturally.

rawhide

  • No package diff

  • Initramfs diff

    -rw-r--r--   1 root     root           27 Dec 31  1969 usr/lib/ostree/prepare-root.conf
    -rw-r--r--   1 root     root           52 Dec 31  1969 usr/lib/ostree/prepare-root.conf
    

    This is from prepare-root.conf now also including the sysroot.readonly, which should also squash this warning from ostree-prepare-root:

    ostree-prepare-root[1319]: Found legacy sysroot.readonly flag, not configured in ostree/prepare-root.conf

    (Compare what we ship to what's in tier-0.)

  • OSTree diff:

    $ ostree diff 42.20240922.dev.0 42.20240922.dev.1
    M    /usr/etc/pki/ca-trust/extracted/java/cacerts
    M    /usr/lib/os-release
    M    /usr/lib/modules/6.12.0-0.rc0.20240920gitbaeb9a7d8b60.7.fc42.x86_64/initramfs.img
    M    /usr/lib/ostree/prepare-root.conf
    M    /usr/lib/sysimage/rpm-ostree-base-db/rpmdb.sqlite
    M    /usr/lib/tmpfiles.d/provision.conf
    M    /usr/share/rpm/rpmdb.sqlite
    M    /usr/share/rpm-ostree/treefile.json
    A    /usr/bin/systemd-firstboot
    A    /usr/lib/.build-id/df/db1101b9d05b622ff69b938d29783028f69ad5
    A    /usr/lib/dracut/dracut.conf.d/20-bootc-base.conf
    A    /usr/lib/dracut/dracut.conf.d/22-bootc-generic.conf
    A    /usr/lib/dracut/dracut.conf.d/49-bootc-tpm2-tss.conf
    A    /usr/lib/dracut/dracut.conf.d/59-altfiles.conf
    A    /usr/lib/systemd/system/systemd-firstboot.service
    

    These are all expected. See related commit for the systemd-firstboot-related one. The dracut dropins are redundant with our current settings, so no change there. The prepare-root.conf change is explained above. The others churn naturally.

@jlebon
Copy link
Member Author

jlebon commented Sep 23, 2024

Huh, looks like the submodule isn't getting initialized in CI. Investigating.
Edit: coreos/coreos-ci-lib#160

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

Thanks for working on this.

# Firewall manipulation
- iptables nftables
- nftables
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised iptables is included in bootc tier-x, but nftables isn't.

Copy link
Member Author

Choose a reason for hiding this comment

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

nftables actually is in tier-0 (and so also tier-x), as a dependency of the container stack. We could make it explicit there to make that clearer?

Copy link
Member

Choose a reason for hiding this comment

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

I think that would be a good idea.

# This manifest can go away in Fedora 42. It duplicates tier-x.

# Default to `bash` in our container, the same as other containers we ship.
# Note this changes to /sbin/init in f42 as inherited by tier-x.
Copy link
Member

Choose a reason for hiding this comment

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

does this make the generated container not podman run able without specifying more on the command line?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

Though more precisely it is still podman run able it just runs systemd and not a shell...

@@ -5,9 +5,6 @@
# One good model is to add fedora-coreos-config as a git submodule. See:
# https://github.com/coreos/coreos-assembler/pull/639

# Include rpm-ostree + kernel + bootloader
include: bootable-rpm-ostree.yaml

Copy link
Member

Choose a reason for hiding this comment

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

notable that fedora-coreos-base.yaml isn't included in RHCOS, but ignition-and-ostree.yaml is. So there will be some fixups to that repo needed when we bump the f-c-c git submodule there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, there's a bunch of tweaks needed in openshift/os to adapt to this. I'm working on that in parallel with this PR but haven't published it yet.

manifests/ignition-and-ostree.yaml Outdated Show resolved Hide resolved
tmp-is-dir: true

# Required by Ignition, and makes the system not compatible with Anaconda
machineid-compat: false
Copy link
Member

Choose a reason for hiding this comment

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

This is also in manifests/tier-x.yaml

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is by design. We need to keep this manifest until we only have tier-x. It's only included in the < 42 path.

.gitmodules Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I'm not the biggest fan of a submodule here but I guess that's really the only way to share at this stage. I really don't know of anything better, but a lot of questions to come to mind.

  1. since we sync around things in this repo using config bot, how are updates to the submodule going to work?
  2. we sometimes have our "streams" on different versions of Fedora. Right now stable testing and testing-devel are on F40, next and next-devel are on F41, and rawhide F42. How do we account for this with a submodule. https://gitlab.com/fedora/bootc/base-images.git isn't a linear definition, it has branches like the rest of Fedora.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

since we sync around things in this repo using config bot, how are updates to the submodule going to work?

We should be able to propagate submodule bumps just like any other file. I need to make sure configbot handles this well.

gitlab.com/fedora/bootc/base-images.git isn't a linear definition, it has branches like the rest of Fedora

This is fixed now! The main branch of base-images tracks all active releases (and we can use releasever conditionals there like we do here, but don't yet).

Copy link
Member

Choose a reason for hiding this comment

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

As of recently I started using subtree merges a bit...it has a huge set of advantages here but some disadvantages in that it's way easier to accidentally fork. But the plus side is it is easy to fork and we don't need hacks like "push downstream branch to upstream".

Copy link
Member

Choose a reason for hiding this comment

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

The other way of course we should share is a variant of https://gitlab.com/fedora/bootc/tracker/-/issues/32 where we publish the manifests as a container image and it's required to COPY --from that image when doing a build.

This has its own advantages and disadvantages.

@cgwalters
Copy link
Member

Sounds like this one isn't blocked, just needs a rebase and socializing?

@jlebon
Copy link
Member Author

jlebon commented Oct 30, 2024

This was unblocked by https://gitlab.com/fedora/bootc/base-images/-/merge_requests/63. I'm planning to get back to it soon.

@jlebon jlebon force-pushed the pr/inherit-tier-x-rawhide branch from d1dd8ef to 0009012 Compare November 7, 2024 20:20
@jlebon
Copy link
Member Author

jlebon commented Nov 7, 2024

Rebased this now that https://gitlab.com/fedora/bootc/tracker/-/issues/39 is resolved and updated for comments!

Also started tracking some of the outstanding items in #3177 (comment).

@jlebon jlebon mentioned this pull request Nov 8, 2024
@jlebon jlebon force-pushed the pr/inherit-tier-x-rawhide branch from 0009012 to dd7ef6e Compare November 14, 2024 01:09
There is a new tier-x in the fedora-bootc project whose goal is to
provide a common base that all variants (including tier-1) can share.

Move FCOS over to use this new tier, but only starting from Fedora 42.

This is a profound change and the start of an exciting new future! This
formalizes our relationship to other image-mode variants, encouraging us
to innovate and solve problems together in a more direct way.

Put more practically, e.g. bug fixes, new features, or temporary
workarounds that concern all/most tier-x derivatives should probably be
carried out at the tier-x level rather than the CoreOS level.

Eventually, this inheritance will be made even more explicit by having
FCOS be built `FROM` the tier-x image. For now, we share at the manifest
level, which is a stepping stone towards that goal.

Patches to actually dedupe our manifests with tier-x will follow. Though
note there is no change in the resulting package set here.
All these manifests are already part of tier-x, so conditionalize them
on <f42.
Split all the things redundant with tier-x into a separate manifest that
only gets inherited on <f42.
Split all the things redundant with tier-x into a separate manifest that
only gets inherited on <f42.
Split all the things redundant with tier-x into a separate manifest that
only gets inherited on <f42.
No functional change. Prep for future patch.
No functional change. Prep for future patch.
This is (or will soon be) part of what defines the base set of packages
needed for a functioning system.

But also this is prep for making this manifest be only included in <f42.
All the requests in this manifest are part of tier-x, so conditionalize
on <f42.
In the spirit of abusing `remove-from-packages` less and being more
closely aligned with tier-x, stop literally nuking the systemd-firstboot
binary and systemd unit. Instead, just disable the systemd unit.

Note it's also disabled in the presets (in `40-coreos-systemd.preset`)
and as mentioned in the comment, via a runtime dropin. So that's triple
disablement already.

But to be sure, also add a non-exclusive test to ensure it doesn't come
back.
Split all the things redundant with tier-x into a separate manifest that
only gets inherited on <f42.
No functional change. This just makes it easier to share the list of
dupes with RHCOS downstream.
This workflow bumps the fedora-bootc submodule. It's better than just
using Dependabot because it only checks for changes in tier-0 and
tier-x. AFAICT, there isn't a way to tell Dependabot to do this.
Currently, when building the squashfs, we drop the `sysroot.readonly`
flag from the ostree config because `ostree-prepare-root` doesn't know
how to handle it in the setup we have in our live environments.

But now inheriting from tier-x, we also inherited the move of that
knob to `/usr/lib/ostree/prepare-root.conf`, which is then included in
the initramfs. That's much harder to override during the build process
because we don't want to rebuild the initramfs. We could probably
instead just append a CPIO to the live initramfs that shadows it but the
real fix anyway is to adapt libostree to work in live environments.[^1]

So for now, just undo this bit to go back to how it was set up before
inheriting from tier-x, where the only sysroot.readonly knob lives in
the ostree repo config.

[^1]: ostreedev/ostree#1921
@jlebon jlebon force-pushed the pr/inherit-tier-x-rawhide branch from dd7ef6e to d533ecb Compare November 14, 2024 21:45
@jlebon jlebon changed the base branch from rawhide to testing-devel November 14, 2024 21:46
@jlebon jlebon marked this pull request as ready for review November 14, 2024 21:46
@jlebon
Copy link
Member Author

jlebon commented Nov 14, 2024

OK, this is ready now! Confirmed that config-bot can handle git submodules fine. Also got the openshift/os changes ready to go, but it'll need to be bundled with the f-c-c submodule bump that brings this change in. For reference, the change is just:

diff --git a/common.yaml b/common.yaml
index d9c9c3f..6df06c8 100644
--- a/common.yaml
+++ b/common.yaml
@@ -7,7 +7,7 @@ include:
   - fedora-coreos-config/manifests/networking-tools.yaml
   - fedora-coreos-config/manifests/user-experience.yaml
   - fedora-coreos-config/manifests/shared-workarounds.yaml
-  - fedora-coreos-config/manifests/bootupd.yaml
+  - fedora-coreos-config/manifests/tier-x.yaml
   # RHCOS owned packages
   - packages-rhcos.yaml

@@ -51,6 +51,9 @@ conditional-include:

 documentation: false

+# historical default
+recommends: true
+
 postprocess:
   # Mark the OS as of the CoreOS variant.
   # XXX: should be part of a centos/redhat-release subpackage instead

CI green against rawhide. Changed target branch to testing-devel.

@jlebon
Copy link
Member Author

jlebon commented Nov 14, 2024

On the RHCOS side, we should wait until 4.18 branches at this point to roll this out, not only to recude risk, but also because it brings in composefs, which is targeted for 4.19.

# https://github.com/ostreedev/ostree/issues/1921
# It's awkward to edit arbitrary keyfile configs. Just rewrite it.
if grep -q readonly /usr/lib/ostree/prepare-root.conf; then
grep -q '^4 ' <(wc -l /usr/lib/ostree/prepare-root.conf)
Copy link
Member

Choose a reason for hiding this comment

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

what is this line doing?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's verifying that the canonical file hasn't changed: https://gitlab.com/fedora/bootc/base-images/-/blob/main/tier-0/ostree.yaml

I guess I could've checksummed it instead.

Comment on lines +20 to +25
# For now, rely on the `sysroot.readonly` knob in /ostree/config only.
# Having it in prepare-root.conf too throws off ostree-prepare-root in
# live PXE/ISO and we have no easy way to override it when building those.
# Really, we need to fix libostree + live ISOs to work well together:
# https://github.com/ostreedev/ostree/issues/1921
# It's awkward to edit arbitrary keyfile configs. Just rewrite it.
Copy link
Member

Choose a reason for hiding this comment

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

I guess my question here is what is the plan to get away from doing this other than fixing libostree + live ISOs to work well together because I'm not confident that's going to be a priority.

@jlebon
Copy link
Member Author

jlebon commented Nov 18, 2024

Did a final sanity-check of the f41 diff:

diff --git a/41.20241118.dev.1/usr/bin/systemd-firstboot b/41.20241118.dev.1/usr/bin/systemd-firstboot
new file mode 100755
index 0000000..685fba0
Binary files /dev/null and b/41.20241118.dev.1/usr/bin/systemd-firstboot differ
diff --git a/41.20241118.dev.0/usr/etc/pki/ca-trust/extracted/java/cacerts b/41.20241118.dev.1/usr/etc/pki/ca-trust/extracted/java/cacerts
index ee9ab18..50e8ac1 100644
Binary files a/41.20241118.dev.0/usr/etc/pki/ca-trust/extracted/java/cacerts and b/41.20241118.dev.1/usr/etc/pki/ca-trust/extracted/java/cacerts differ
diff --git a/41.20241118.dev.1/usr/lib/.build-id/e7/95e9082536fa7561c41a4e02fd761dd69d91af b/41.20241118.dev.1/usr/lib/.build-id/e7/95e9082536fa7561c41a4e02fd761dd69d91af
new file mode 120000
index 0000000..c52f669
--- /dev/null
+++ b/41.20241118.dev.1/usr/lib/.build-id/e7/95e9082536fa7561c41a4e02fd761dd69d91af
@@ -0,0 +1 @@
+../../../../usr/bin/systemd-firstboot
\ No newline at end of file
diff --git a/41.20241118.dev.0/usr/lib/modules/6.11.7-300.fc41.x86_64/initramfs.img b/41.20241118.dev.1/usr/lib/modules/6.11.7-300.fc41.x86_64/initramfs.img
index 95e53d8..df219d7 100644
Binary files a/41.20241118.dev.0/usr/lib/modules/6.11.7-300.fc41.x86_64/initramfs.img and b/41.20241118.dev.1/usr/lib/modules/6.11.7-300.fc41.x86_64/initramfs.img differ
diff --git a/41.20241118.dev.0/usr/lib/os-release b/41.20241118.dev.1/usr/lib/os-release
index c6d5e2c..1606c54 100644
--- a/41.20241118.dev.0/usr/lib/os-release
+++ b/41.20241118.dev.1/usr/lib/os-release
@@ -1,11 +1,11 @@
 NAME="Fedora Linux"
-VERSION="41.20241118.dev.0 (CoreOS)"
+VERSION="41.20241118.dev.1 (CoreOS)"
 RELEASE_TYPE=stable
 ID=fedora
 VERSION_ID=41
 VERSION_CODENAME=""
 PLATFORM_ID="platform:f41"
-PRETTY_NAME="Fedora CoreOS 41.20241118.dev.0"
+PRETTY_NAME="Fedora CoreOS 41.20241118.dev.1"
 ANSI_COLOR="0;38;2;60;110;180"
 LOGO=fedora-logo-icon
 CPE_NAME="cpe:/o:fedoraproject:fedora:41"
@@ -20,4 +20,4 @@ REDHAT_SUPPORT_PRODUCT_VERSION=41
 SUPPORT_END=2025-12-15
 VARIANT="CoreOS"
 VARIANT_ID=coreos
-OSTREE_VERSION='41.20241118.dev.0'
+OSTREE_VERSION='41.20241118.dev.1'
diff --git a/41.20241118.dev.0/usr/lib/sysimage/rpm-ostree-base-db/rpmdb.sqlite b/41.20241118.dev.1/usr/lib/sysimage/rpm-ostree-base-db/rpmdb.sqlite
index a3a8689..dcfc2f7 100644
Binary files a/41.20241118.dev.0/usr/lib/sysimage/rpm-ostree-base-db/rpmdb.sqlite and b/41.20241118.dev.1/usr/lib/sysimage/rpm-ostree-base-db/rpmdb.sqlite differ
diff --git a/41.20241118.dev.1/usr/lib/systemd/system/systemd-firstboot.service b/41.20241118.dev.1/usr/lib/systemd/system/systemd-firstboot.service
new file mode 100644
index 0000000..78a4087
--- /dev/null
+++ b/41.20241118.dev.1/usr/lib/systemd/system/systemd-firstboot.service
@@ -0,0 +1,45 @@
+#  SPDX-License-Identifier: LGPL-2.1-or-later
+#
+#  This file is part of systemd.
+#
+#  systemd is free software; you can redistribute it and/or modify it
+#  under the terms of the GNU Lesser General Public License as published by
+#  the Free Software Foundation; either version 2.1 of the License, or
+#  (at your option) any later version.
+
+[Unit]
+Description=First Boot Wizard
+Documentation=man:systemd-firstboot(1)
+
+ConditionPathIsReadWrite=/etc
+ConditionFirstBoot=yes
+
+DefaultDependencies=no
+# This service may need to write to the file system:
+After=systemd-remount-fs.service
+# Both systemd-sysusers and systemd-tmpfiles may create the root account
+# (via factory files or credentials), obviating the need for us to do that:
+After=systemd-sysusers.service systemd-tmpfiles-setup.service
+# Let vconsole-setup do its setup before starting user interaction:
+After=systemd-vconsole-setup.service
+
+Wants=first-boot-complete.target
+Before=first-boot-complete.target sysinit.target
+Conflicts=shutdown.target
+Before=shutdown.target
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=systemd-firstboot --prompt-locale --prompt-timezone --prompt-root-password
+StandardOutput=tty
+StandardInput=tty
+StandardError=tty
+
+# Optionally, pick up basic fields from credentials passed to the service
+# manager. This is useful for importing this data from nspawn's
+# --set-credential= switch.
+ImportCredential=passwd.hashed-password.root
+ImportCredential=passwd.plaintext-password.root
+ImportCredential=passwd.shell.root
+ImportCredential=firstboot.*
diff --git a/41.20241118.dev.0/usr/share/rpm/rpmdb.sqlite b/41.20241118.dev.1/usr/share/rpm/rpmdb.sqlite
index a3a8689..dcfc2f7 100644
Binary files a/41.20241118.dev.0/usr/share/rpm/rpmdb.sqlite and b/41.20241118.dev.1/usr/share/rpm/rpmdb.sqlite differ
diff --git a/41.20241118.dev.0/usr/share/rpm-ostree/treefile.json b/41.20241118.dev.1/usr/share/rpm-ostree/treefile.json
index 81cfe0b..9fd2a96 100644
--- a/41.20241118.dev.0/usr/share/rpm-ostree/treefile.json
+++ b/41.20241118.dev.1/usr/share/rpm-ostree/treefile.json
@@ -246,8 +246,6 @@
     ],
     [
       "systemd",
-      "/usr/bin/systemd-firstboot",
-      "/usr/lib/systemd/system/systemd-firstboot.service",
       "/usr/lib/systemd/system/sysinit.target.wants/systemd-firstboot.service"
     ],
     [

So basically, the main difference is that systemd-firstboot is no longer nuked (but again, it's still disabled).

@jlebon jlebon merged commit 346724f into coreos:testing-devel Nov 18, 2024
1 check passed
@travier
Copy link
Member

travier commented Nov 19, 2024

Hum, RHCOS failed on the booupd metadata generation: openshift/os#1644 (comment)

Not completely sure if it's related to this one yet.

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

Successfully merging this pull request may close these issues.

4 participants