search

cpanel / elevate

elevate your cPanel&WHM CentOS 7 server to Almalinux 8
https://cpanel.github.io/elevate/
BSD 2-Clause "Simplified" License
45 stars 30 forks source link

Check whether leapp can control the boot process #479

Closed sloanebernstein closed 3 months ago

sloanebernstein commented 3 months ago

Case RE-514: Currently, the script assumes that leapp will have sufficient control over the boot process via GRUB2 configuration. This can result in upgrades making it all the way to leapp only to discover that leapp cannot boot into the custom environment leapp uses to carry out the upgrade, leading to a system that largely cannot be restored into a fully functional state.

It is possible to check that changes made to the bootloader are reflected in the behavior of the boot process, but this requires doing work prior to any major changes to the system. Stage 1 is an ideal time for this, but also stage 1 was not designed to result in a reboot, unlike every other stage. To make this work, the bootstraping of the elevate-cpanel service has been moved to a pre-stage, and the start and continuation entry points have been changed not to call stage 1 directly. This allows the stage 1 subroutine to return a signal to reboot like all the other stages, though it also results in some cosmetic and (hopefully) minor changes to user-facing behavior, some of which can be mitigated.

In particular, the check being done is to use the grubby utility to add and later remove a kernel argument in the default boot entry. The argument is designed to be exceedingly unlikely to ever be something that the kernel or PID 1 would recognize, and thus they ignore it; the script then checks for the argument via /proc/cmdline at the very start of stage 2. If found, the rest of stage 2 proceeds as normal; if not, the entire ELevate process is reset as if --start had never been passed to the script, and a custom failure notification is emitted.

Changelog: Check whether leapp can control the boot process before significant and irreversable changes are applied to the system.

By submitting pull requests to this repo, I agree to the Contributor License Agreement which can be found at: https://github.com/cpanel/elevate/blob/main/docs/cPanel-CLA.pdf

sloanebernstein commented 3 months ago

Overall considering the scope of the change I'd normally say "I'm surprised this did not cause tests to fail," but I know well the state of the t/ dir. This is however the perfect time to add tests for the updated/added functionality. I leave it to your discretion as to whether you have time for that or not, but strongly recommend it.

There are basically no tests for the high-level logic and control flow of the script, and it's not exactly written in a way that is conducive to making that happen. Tests for the Grub2 component should be reasonable, though.

troglodyne commented 3 months ago

Added test looks good to me, probably time to take another look @cPholloway