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

Integrate with kernel-install.d #4726

Open
cgwalters opened this issue Dec 12, 2023 · 5 comments · May be fixed by #5135
Open

Integrate with kernel-install.d #4726

cgwalters opened this issue Dec 12, 2023 · 5 comments · May be fixed by #5135
Assignees

Comments

@cgwalters
Copy link
Member

cgwalters commented Dec 12, 2023

Right now our code dealing with the Fedora-derivatives kernel packaging is a mess. I think what would work well is to add /usr/lib/kernel/05-rpmostree.install and:

Detect if the system is actively (rpm-)ostree based (otherwise we could cause problems if people just happen to have the package installed...I think for now the important case is /run/ostree-booted for hosts and detecting our containers); if it's not we just exit 0.

Otherwise, looking at all the stuff in

# ls -al /usr/lib/kernel/install.d/
total 40
drwxr-xr-x. 1 root root   34 Dec 12 16:24 .
drwxr-xr-x. 1 root root   18 Dec 12 16:24 ..
-rwxr-xr-x. 2 root root 8053 Jan  1  1970 20-grub.install
-rwxr-xr-x. 2 root root 1905 Jan  1  1970 20-grubby.install
-rwxr-xr-x. 2 root root 2006 Jan  1  1970 50-depmod.install
-rwxr-xr-x. 2 root root 1895 Jan  1  1970 50-dracut.install
-rwxr-xr-x. 2 root root  791 Jan  1  1970 60-kdump.install
-rwxr-xr-x. 2 root root 5635 Jan  1  1970 90-loaderentry.install
-rwxr-xr-x. 2 root root  204 Jan  1  1970 92-crashkernel.install
-rwxr-xr-x. 2 root root 1989 Jan  1  1970 99-grub-mkconfig.install

I think except for the crashkernel bits we handle all of that internally and should basically fork off rpm-ostree kernel-install add (we have some cliwrap code for this already).

A crucial detail here is that the docs say if a script does an exit 77 then everything else is skipped, which is what we need.

If we do that well, I think we might be able drop our logic for "ignore kernel postinsts" which would be really nice...well almost except it seems that the kernel posts still directly run dracut...argh. (What is the 50-dracut.install doing then?)

However, there's some important bits we should handle in the container case specifically. With this we'd be really close to having dnf update kernel work - except we need (well, want) to do cleanup like:

  • removing the copies of kernel data in /boot
  • Ensuring there's only one kernel installed
@cgwalters
Copy link
Member Author

cgwalters commented Dec 12, 2023

In some experimentation, this is getting pretty close:

$ cat /usr/lib/kernel/install.d/07-rpmostree.install
#!/usr/bin/bash

set -euo pipefail

COMMAND="${1:?}"
KERNEL_VERSION="${2:?}"

[ -w "/lib/modules" ] || exit 0
[ -d "/sysroot/ostree/repo" ] || exit 0

# Call hooks that we want
/usr/lib/kernel/install.d/50-depmod.install "${COMMAND}" "${KERNEL_VERSION}"

case "$COMMAND" in
    add)
        test -n "${KERNEL_VERSION}"
        # We don't need this stuff
        rm -f /boot/*${KERNEL_VERSION}*
        dracut -v -f /usr/lib/modules/${KERNEL_VERSION}/initramfs.img "${KERNEL_VERSION}"
        # And ensure nothing else runs
        exit 77
        ;;
    remove)
        rm -f "/usr/lib/modules/${KERNEL_VERSION}/initramfs.img"
        exit 77
        ;;
    *)
        exit 0
        ;;
esac

@ericcurtin
Copy link
Contributor

From the Automotive perspective, a lot of the people in the community at the moment are kernel hackers, as there is plenty of hardware enablement to do in ARM in general.

@dougsland
Copy link
Contributor

In some experimentation, this is getting pretty close:

$ cat /usr/lib/kernel/install.d/07-rpmostree.install
#!/usr/bin/bash

set -euo pipefail

COMMAND="${1:?}"
KERNEL_VERSION="${2:?}"

[ -w "/lib/modules" ] || exit 0
[ -d "/sysroot/ostree/repo" ] || exit 0

# Call hooks that we want
/usr/lib/kernel/install.d/50-depmod.install "${COMMAND}" "${KERNEL_VERSION}"

case "$COMMAND" in
    add)
        test -n "${KERNEL_VERSION}"
        # We don't need this stuff
        rm -f /boot/*${KERNEL_VERSION}*
        dracut -v -f /usr/lib/modules/${KERNEL_VERSION}/initramfs.img "${KERNEL_VERSION}"
        # And ensure nothing else runs
        exit 77
        ;;
    remove)
        rm -f "/usr/lib/modules/${KERNEL_VERSION}/initramfs.img"
        exit 77
        ;;
    *)
        exit 0
        ;;
esac

@cgwalters LGTM, great patch. Just a small suggestion, could you please use a bash constant for the exit 77? I understood this is "common" usage in the similar scripts but what this signal in the end? My guess is: "all tasks for the command parameters are done and success" instead of exit 0 (all script run just fine, success). Linking a documentation here would even be better. :)

jlebon added a commit to jlebon/centos-bootc that referenced this issue Feb 29, 2024
Until the native kernel-install is OSTree and container aware[[1]], we
need to have a custom `kernel-install` for kernel replacements to work
in a derived build.

This will make it so that users don't have to first install the wrappers
themselves in their Containerfiles[[2]].

The caveat with this is that if anywhere in the derivation, systemd is
updated, the update will retake ownership of `/usr/bin/kernel-install`,
which means that a further kernel replacement down the derivation chain
would hit issues. We can document that. This still seems worth the UX
improvement in the common case.

[1]: coreos/rpm-ostree#4726
[2]: https://github.com/coreos/layering-examples/blob/9329023b/replace-kernel/Containerfile#L4
jlebon added a commit to jlebon/centos-bootc that referenced this issue Feb 29, 2024
Until the native kernel-install is OSTree and container aware[[1]], we
need to have a custom `kernel-install` for kernel replacements to work
in a derived build.

This will make it so that users don't have to first install the wrappers
themselves in their Containerfiles[[2]].

The caveat with this is that if anywhere in the derivation, systemd is
updated, the update will retake ownership of `/usr/bin/kernel-install`,
which means that a further kernel replacement down the derivation chain
would hit issues. We can document that. This still seems worth the UX
improvement in the common case.

[1]: coreos/rpm-ostree#4726
[2]: https://github.com/coreos/layering-examples/blob/9329023b/replace-kernel/Containerfile#L4
jlebon added a commit to jlebon/centos-bootc that referenced this issue Feb 29, 2024
Until the native kernel-install is OSTree and container aware[[1]], we
need to have a custom `kernel-install` for kernel replacements to work
in a derived build.

This will make it so that users don't have to first install the wrappers
themselves in their Containerfiles[[2]].

The caveat with this is that if anywhere in the derivation, systemd is
updated, the update will retake ownership of `/usr/bin/kernel-install`,
which means that a further kernel replacement down the derivation chain
would hit issues. We can document that. This still seems worth the UX
improvement in the common case.

[1]: coreos/rpm-ostree#4726
[2]: https://github.com/coreos/layering-examples/blob/9329023b/replace-kernel/Containerfile#L4
@jlebon
Copy link
Member

jlebon commented Feb 29, 2024

xref: CentOS/centos-bootc#377

@jlebon
Copy link
Member

jlebon commented May 7, 2024

xref: #4950

cgwalters added a commit to cgwalters/rpm-ostree that referenced this issue Sep 12, 2024
I'm looking at handling kernel overrides:
- coreos#4726
- https://gitlab.com/fedora/bootc/tracker/-/issues/22

And I think the cliwrap thing turned out to be the wrong approach
for this.

There's consensus now it was the wrong approach, so deprecate it.

Signed-off-by: Colin Walters <walters@verbum.org>
cgwalters added a commit to cgwalters/rpm-ostree that referenced this issue Sep 12, 2024
I'm looking at handling kernel overrides:
- coreos#4726
- https://gitlab.com/fedora/bootc/tracker/-/issues/22

And I think the cliwrap thing turned out to be the wrong approach
for this.

There's consensus now it was the wrong approach, so deprecate it.

Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters cgwalters self-assigned this Sep 16, 2024
@jmarrero jmarrero linked a pull request Oct 22, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants