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

manifests: Move kernel into separate manifest #2609

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

cgwalters
Copy link
Member

@cgwalters cgwalters commented Sep 15, 2023

This is prep for rebasing on sagano
where I hit on the issue that to build images with kernel-rt,
we need to clearly separate the kernel package from userspace
stuff (systemd, rpm-ostree) etc.

Note that we'll need to make the same change in RHCOS - until
we rebase both on sagano.


@cgwalters cgwalters changed the title manifests: Move kernel into fedora-coreos-base manifests: Move kernel into fedora-coreos-base && ci: Rework to do validation after fetching git Sep 15, 2023
@cgwalters cgwalters force-pushed the inherit-sagano-prep branch 6 times, most recently from 01a73b4 to fece2dd Compare September 18, 2023 13:20
@jlebon
Copy link
Member

jlebon commented Sep 18, 2023

Hmm, odd that this is required. The kernel-rt-core package should Provide kernel so the kernel requirement here should be satisfied.

What was the depsolve error that you got?

@cgwalters
Copy link
Member Author

The problem is easy to replicate with this patch to the sagano configs:

diff --git a/tier-0/kernel-rt.yaml b/tier-0/kernel-rt.yaml
index cdcff10..e9dad97 100644
--- a/tier-0/kernel-rt.yaml
+++ b/tier-0/kernel-rt.yaml
@@ -4,7 +4,7 @@ repos:
 
 # Enable the "realtime" AKA soft-realtime AKA latency-optimized kernel.
 packages:
- - kernel-rt-core kernel-rt-modules kernel-rt-modules-extra kernel-rt-kvm
+ - kernel kernel-rt-core kernel-rt-modules kernel-rt-modules-extra kernel-rt-kvm
 
 exclude-packages:
   - kernel-rt-debug-core
$ rpm-ostree compose image --cachedir=/builds/compose.cache --initialize-mode if-not-exists --format=oci centos-tier-0-rt-stream9.yaml out.oci
...
error: Finalizing rootfs: During kernel processing: Multiple kernels (vmlinuz) found in: usr/lib/modules: 5.14.0-366.el9.x86_64+rt 5.14.0-366.el9.x86_64

(Clearly we should error out in the depsolve phase, not finalization here if multiple things provide kernels)

@cgwalters
Copy link
Member Author

There are of course multiple other ways I can think of to address this.

The first would be something like

package_slots:
  kernel:
    - kernel

Under our inheritance rules this would allow something else to later do:

package_slots:
  kernel:
    - kernel-rt

and override instead of union.

Even then though, it seems like just a Good Idea to me to separate the userspace configs from the kernel as is being done here.

@cgwalters cgwalters added the area/container-native Work related to moving FCOS to be container-native label Sep 19, 2023
jlebon
jlebon previously approved these changes Sep 21, 2023
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Some comments but looks sane overall.

manifests/fedora-coreos-base.yaml Outdated Show resolved Hide resolved
.cci.jenkinsfile Outdated Show resolved Hide resolved
This is prep for rebasing on [sagano](https://gitlab.com/cgwalters-playground/sagano)
where I hit on the issue that to build images with `kernel-rt`,
we need to clearly separate the `kernel` package from userspace
stuff (`systemd`, `rpm-ostree`) etc.

Note that we'll need to make the same change in RHCOS - until
we rebase both on sagano.
@cgwalters cgwalters changed the title manifests: Move kernel into fedora-coreos-base && ci: Rework to do validation after fetching git manifests: Move kernel into separate manifest Oct 27, 2023
@cgwalters cgwalters enabled auto-merge (rebase) October 27, 2023 12:11
@cgwalters
Copy link
Member Author

OK, addressed comments and queued for merge.

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM!

@cgwalters cgwalters merged commit 6027dbf into coreos:testing-devel Oct 27, 2023
3 checks passed
jlebon added a commit to coreosbot-releng/os that referenced this pull request Nov 9, 2023
This was split out in a separate manifest in
coreos/fedora-coreos-config#2609

We'll probably rejig this soon once we inherit from the bootable
container work, but for now this restore it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/container-native Work related to moving FCOS to be container-native
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants