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.

Sign up for GitHub

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

Jump to bottom

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
@cgwalters
Labels
container-native difficulty/medium medium complexity/difficutly issue kind/enhancement size/L

Comments

@cgwalters
Copy link
Member

cgwalters commented Dec 12, 2023 •
edited
Loading

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 •
edited
Loading

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 cgwalters mentioned this issue Dec 15, 2023
Open
@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. :)

@cgwalters cgwalters mentioned this issue Feb 29, 2024
Closed
jlebon added a commit to jlebon/centos-bootc that referenced this issue Feb 29, 2024
@jlebon
tier-0: enable kernel-install cliwrap
edf1fb7 
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
@jlebon
tier-0: enable kernel-install cliwrap
d3c87de 
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
@jlebon
tier-0: enable kernel-install cliwrap
224ae97 
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 jlebon mentioned this issue Feb 29, 2024
Closed
@jlebon
Copy link
Member

jlebon commented Feb 29, 2024

xref: CentOS/centos-bootc#377

@jlebon jlebon mentioned this issue Mar 5, 2024
Closed
@cgwalters cgwalters mentioned this issue Apr 10, 2024
Merged
@jlebon jlebon mentioned this issue May 7, 2024
Closed
@jlebon
Copy link
Member

jlebon commented May 7, 2024

xref: #4950

@jlebon jlebon mentioned this issue May 7, 2024
Merged
This was referenced May 30, 2024
Open
Open
cgwalters added a commit to cgwalters/rpm-ostree that referenced this issue Sep 12, 2024
@cgwalters
Deprecate cliwrap
4f2506a 
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 <[email protected]>
cgwalters added a commit to cgwalters/rpm-ostree that referenced this issue Sep 12, 2024
@cgwalters
Deprecate cliwrap
242762e 
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 <[email protected]>
@cgwalters cgwalters mentioned this issue Sep 12, 2024
Merged
@cgwalters cgwalters self-assigned this Sep 16, 2024
@cgwalters cgwalters mentioned this issue Sep 16, 2024
Closed
@jmarrero jmarrero linked a pull request Oct 22, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cgwalters cgwalters

Labels
container-native difficulty/medium medium complexity/difficutly issue kind/enhancement size/L
Projects
None yet
Milestone
No milestone
4 participants
@cgwalters @dougsland @ericcurtin @jlebon