From 8e690d56781c017858596c95e6aa43bb775ca2cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Sl=C3=A1vik?= Date: Wed, 22 Nov 2023 13:48:32 +0100 Subject: [PATCH 1/3] ostree: Use bootupd if installed by payload Original commit message from Colin Walters was: The https://github.com/coreos/bootupd project was created to fill the gap in bootloader management for ostree-based systems. When it was created, it was just integrated into Fedora CoreOS and derivatives; this left the Atomic Desktops (Silverblue etc.) as unfixed, and it was never used by RHEL for Edge. This PR is aiming to circle back and close that gap. We detect if bootupd is in the target root; if it is, then we skip the regular bootloader work, and just run bootupd to perform the installation. The other hacks we have around the grub config are no longer necessary in this mode. --- .../payload/rpm_ostree/installation.py | 32 +++++++++++++- .../payloads/payload/rpm_ostree/util.py | 27 ++++++++++++ .../payloads/payload/test_rpm_ostree_tasks.py | 42 ++++++++++++++++++- .../payloads/payload/test_rpm_ostree_util.py | 34 +++++++++++++++ 4 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 pyanaconda/modules/payloads/payload/rpm_ostree/util.py create mode 100644 tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_rpm_ostree_util.py diff --git a/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py b/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py index 67d144235d3..bab8ce2e5d3 100644 --- a/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py +++ b/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py @@ -26,11 +26,13 @@ from pyanaconda.core.i18n import _ from pyanaconda.core.path import set_system_root, make_directories from pyanaconda.core.util import execWithRedirect -from pyanaconda.modules.common.errors.installation import PayloadInstallationError +from pyanaconda.modules.common.errors.installation import PayloadInstallationError, \ + BootloaderInstallationError from pyanaconda.modules.common.task import Task from pyanaconda.modules.common.constants.objects import DEVICE_TREE, BOOTLOADER from pyanaconda.modules.common.constants.services import STORAGE from pyanaconda.modules.common.structures.storage import DeviceData +from pyanaconda.modules.payloads.payload.rpm_ostree.util import have_bootupd import gi gi.require_version("OSTree", "1.0") @@ -496,9 +498,35 @@ def name(self): return "Configure OSTree bootloader" def run(self): - self._move_grub_config() + if have_bootupd(self._sysroot): + self._install_bootupd() + else: + self._move_grub_config() self._set_kargs() + def _install_bootupd(self): + bootloader = STORAGE.get_proxy(BOOTLOADER) + device_tree = STORAGE.get_proxy(DEVICE_TREE) + dev_data = DeviceData.from_structure(device_tree.GetDeviceData(bootloader.Drive)) + + rc = execWithRedirect( + "bootupctl", + [ + "backend", + "install", + "--auto", + "--with-static-configs", + "--device", + dev_data.path, + "/", + ], + root=self._sysroot + ) + + if rc: + raise BootloaderInstallationError( + "failed to write boot loader configuration") + def _move_grub_config(self): """If using GRUB2, move its config file, also with a compatibility symlink.""" boot_grub2_cfg = self._sysroot + '/boot/grub2/grub.cfg' diff --git a/pyanaconda/modules/payloads/payload/rpm_ostree/util.py b/pyanaconda/modules/payloads/payload/rpm_ostree/util.py new file mode 100644 index 00000000000..d604dcd0ff9 --- /dev/null +++ b/pyanaconda/modules/payloads/payload/rpm_ostree/util.py @@ -0,0 +1,27 @@ +# +# Copyright (C) 2023 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# + +import os.path +from pyanaconda.core.path import join_paths + +__all__ = ["have_bootupd"] + + +def have_bootupd(sysroot): + """Is bootupd/bootupctl present in sysroot?""" + return os.path.exists(join_paths(sysroot, "/usr/bin/bootupctl")) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_rpm_ostree_tasks.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_rpm_ostree_tasks.py index b0914af7142..a146687fb75 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_rpm_ostree_tasks.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_rpm_ostree_tasks.py @@ -32,7 +32,6 @@ ChangeOSTreeRemoteTask, ConfigureBootloader, DeployOSTreeTask, PullRemoteAndDeleteTask, \ SetSystemRootTask, TearDownOSTreeMountTargetsTask - def _make_config_data(): """Create OSTree configuration data for testing @@ -748,6 +747,47 @@ def test_nonbtrfs_run(self, devdata_mock, storage_mock, symlink_mock, rename_moc root=sysroot ) + @patch("pyanaconda.modules.payloads.payload.rpm_ostree.installation.have_bootupd") + @patch("pyanaconda.modules.payloads.payload.rpm_ostree.installation.execWithRedirect") + @patch("pyanaconda.modules.payloads.payload.rpm_ostree.installation.os.rename") + @patch("pyanaconda.modules.payloads.payload.rpm_ostree.installation.os.symlink") + @patch("pyanaconda.modules.payloads.payload.rpm_ostree.installation.STORAGE") + @patch("pyanaconda.modules.payloads.payload.rpm_ostree.installation.DeviceData") + def test_bootupd_run(self, devdata_mock, storage_mock, symlink_mock, rename_mock, exec_mock, + have_bootupd_mock): + """Test OSTree bootloader config task, bootupd""" + exec_mock.return_value = 0 + have_bootupd_mock.return_value = True + + proxy_mock = storage_mock.get_proxy() + proxy_mock.GetArguments.return_value = ["BOOTLOADER-ARGS"] + proxy_mock.GetFstabSpec.return_value = "FSTAB-SPEC" + proxy_mock.GetRootDevice.return_value = "device-name" + proxy_mock.Drive = "btldr-drv" + devdata_mock.from_structure.return_value.type = "something-non-btrfs-subvolume-ish" + devdata_mock.from_structure.return_value.path = "/dev/btldr-drv" + + with tempfile.TemporaryDirectory() as sysroot: + task = ConfigureBootloader(sysroot) + task.run() + + rename_mock.assert_not_called() + symlink_mock.assert_not_called() + assert exec_mock.call_count == 2 + exec_mock.assert_has_calls([ + call( + "bootupctl", + ["backend", "install", "--auto", "--with-static-configs", "--device", + "/dev/btldr-drv", "/"], + root=sysroot + ), + call( + "ostree", + ["admin", "instutil", "set-kargs", "BOOTLOADER-ARGS", "root=FSTAB-SPEC", "rw"], + root=sysroot + ) + ]) + @patch("pyanaconda.modules.payloads.payload.rpm_ostree.installation.execWithRedirect") @patch("pyanaconda.modules.payloads.payload.rpm_ostree.installation.os.rename") @patch("pyanaconda.modules.payloads.payload.rpm_ostree.installation.os.symlink") diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_rpm_ostree_util.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_rpm_ostree_util.py new file mode 100644 index 00000000000..df77041d92a --- /dev/null +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_rpm_ostree_util.py @@ -0,0 +1,34 @@ +# +# Copyright (C) 2020 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# +import tempfile +import unittest +from pyanaconda.core.path import join_paths, touch, make_directories + +from pyanaconda.modules.payloads.payload.rpm_ostree.util import have_bootupd + +class RPMOSTreeUtilTestCase(unittest.TestCase): + """Test the RPM OSTree utils.""" + + def test_have_bootupd(self): + """Test bootupd detection.""" + with tempfile.TemporaryDirectory() as sysroot: + assert have_bootupd(sysroot) is False + + make_directories(join_paths(sysroot, "/usr/bin")) + touch(join_paths(sysroot, "/usr/bin/bootupctl")) + assert have_bootupd(sysroot) is True From 0d42d2fac55d2355e98334de08494dc0eb3d5a0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Sl=C3=A1vik?= Date: Wed, 22 Nov 2023 13:35:07 +0100 Subject: [PATCH 2/3] bootloader: Detect bootupd and skip regular install --- .../modules/storage/bootloader/bootloader.py | 4 +- .../storage/bootloader/installation.py | 13 +++- .../modules/storage/test_module_bootloader.py | 61 ++++++++++++++++--- 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/pyanaconda/modules/storage/bootloader/bootloader.py b/pyanaconda/modules/storage/bootloader/bootloader.py index 7d7c9394247..b8d41416dd7 100644 --- a/pyanaconda/modules/storage/bootloader/bootloader.py +++ b/pyanaconda/modules/storage/bootloader/bootloader.py @@ -475,7 +475,9 @@ def install_bootloader_with_tasks(self, payload_type, kernel_versions): ), InstallBootloaderTask( storage=self.storage, - mode=self.bootloader_mode + mode=self.bootloader_mode, + payload_type=payload_type, + sysroot=conf.target.system_root ), CreateBLSEntriesTask( storage=self.storage, diff --git a/pyanaconda/modules/storage/bootloader/installation.py b/pyanaconda/modules/storage/bootloader/installation.py index 5509f09b602..7cb1e8ba52c 100644 --- a/pyanaconda/modules/storage/bootloader/installation.py +++ b/pyanaconda/modules/storage/bootloader/installation.py @@ -20,6 +20,7 @@ from blivet import arch from blivet.devices import BTRFSDevice from pyanaconda.core.constants import PAYLOAD_TYPE_RPM_OSTREE, PAYLOAD_LIVE_TYPES +from pyanaconda.modules.payloads.payload.rpm_ostree.util import have_bootupd from pyanaconda.modules.storage.bootloader import BootLoaderError from pyanaconda.modules.storage.bootloader.systemd import SystemdBoot from pyanaconda.core.util import execWithRedirect @@ -152,11 +153,13 @@ def run(self): class InstallBootloaderTask(Task): """Installation task for the bootloader.""" - def __init__(self, storage, mode): + def __init__(self, storage, mode, payload_type, sysroot): """Create a new task.""" super().__init__() self._storage = storage self._mode = mode + self._payload_type = payload_type + self._sysroot = sysroot @property def name(self): @@ -185,6 +188,10 @@ def run(self): log.debug("The bootloader installation is skipped.") return + if self._payload_type == PAYLOAD_TYPE_RPM_OSTREE and have_bootupd(self._sysroot): + log.debug("Will not install regular bootloader for ostree with bootupd") + return + log.debug("Installing the boot loader.") try: @@ -306,7 +313,9 @@ def run(self): InstallBootloaderTask( self._storage, - self._mode + self._mode, + self._payload_type, + self._sysroot ).run() diff --git a/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_bootloader.py b/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_bootloader.py index 5694b7b8d2d..09dd9bf424c 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_bootloader.py +++ b/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_bootloader.py @@ -33,6 +33,7 @@ from tests.unit_tests.pyanaconda_tests import patch_dbus_publish_object, check_dbus_property, \ reset_boot_loader_factory, check_task_creation_list, check_task_creation +from pyanaconda.core.path import make_directories, touch from pyanaconda.modules.storage import platform from pyanaconda.modules.storage.bootloader import BootLoaderFactory from pyanaconda.modules.storage.bootloader.base import BootLoader @@ -45,7 +46,7 @@ from pyanaconda.modules.storage.bootloader.image import LinuxBootLoaderImage from pyanaconda.core.constants import BOOTLOADER_SKIPPED, BOOTLOADER_LOCATION_PARTITION, \ - PAYLOAD_TYPE_RPM_OSTREE, PAYLOAD_TYPE_LIVE_IMAGE + PAYLOAD_TYPE_RPM_OSTREE, PAYLOAD_TYPE_LIVE_IMAGE, PAYLOAD_TYPE_DNF from pyanaconda.modules.common.constants.objects import BOOTLOADER from pyanaconda.modules.storage.bootloader import BootloaderModule from pyanaconda.modules.storage.bootloader.bootloader_interface import BootloaderInterface @@ -395,15 +396,55 @@ def test_install(self): bootloader = Mock() storage = Mock(bootloader=bootloader) - InstallBootloaderTask(storage, BootloaderMode.DISABLED).run() - bootloader.write.assert_not_called() + with tempfile.TemporaryDirectory() as sysroot: + InstallBootloaderTask( + storage, + BootloaderMode.DISABLED, + PAYLOAD_TYPE_DNF, + sysroot + ).run() + bootloader.write.assert_not_called() - InstallBootloaderTask(storage, BootloaderMode.SKIPPED).run() - bootloader.write.assert_not_called() + InstallBootloaderTask( + storage, + BootloaderMode.SKIPPED, + PAYLOAD_TYPE_DNF, + sysroot + ).run() + bootloader.write.assert_not_called() - InstallBootloaderTask(storage, BootloaderMode.ENABLED).run() - bootloader.prepare.assert_called_once() - bootloader.write.assert_called_once() + InstallBootloaderTask( + storage, + BootloaderMode.ENABLED, + PAYLOAD_TYPE_DNF, + sysroot + ).run() + bootloader.prepare.assert_called_once() + bootloader.write.assert_called_once() + + bootloader.prepare.reset_mock() + bootloader.write.reset_mock() + InstallBootloaderTask( + storage, + BootloaderMode.ENABLED, + PAYLOAD_TYPE_RPM_OSTREE, + sysroot + ).run() + bootloader.prepare.assert_called_once() + bootloader.write.assert_called_once() + + bootloader.prepare.reset_mock() + bootloader.write.reset_mock() + make_directories(sysroot + "/usr/bin") + touch(sysroot + "/usr/bin/bootupctl") + InstallBootloaderTask( + storage, + BootloaderMode.ENABLED, + PAYLOAD_TYPE_RPM_OSTREE, + sysroot + ).run() + bootloader.prepare.assert_not_called() + bootloader.write.assert_not_called() @patch('pyanaconda.modules.storage.bootloader.utils.execWithRedirect') def test_create_bls_entries(self, exec_mock): @@ -669,7 +710,9 @@ def test_fix_btrfs(self, configure, install, conf): ) install.assert_called_once_with( storage, - BootloaderMode.ENABLED + BootloaderMode.ENABLED, + PAYLOAD_TYPE_LIVE_IMAGE, + sysroot ) @patch('pyanaconda.modules.storage.bootloader.installation.conf') From af31179e5001b82dad41ef9f7ae9e59753b3b86f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Sl=C3=A1vik?= Date: Thu, 23 Nov 2023 15:14:47 +0100 Subject: [PATCH 3/3] docs: Add release note for bootupd support --- docs/release-notes/bootupd.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 docs/release-notes/bootupd.rst diff --git a/docs/release-notes/bootupd.rst b/docs/release-notes/bootupd.rst new file mode 100644 index 00000000000..c15be6d166f --- /dev/null +++ b/docs/release-notes/bootupd.rst @@ -0,0 +1,10 @@ +:Type: OSTree +:Summary: Preliminary support for bootable ostree containers + +:Description: + Anaconda can now correctly detect and use the bootupd bootloader used in + bootable ostree containers. When the installed container includes the ``bootupctl`` tool, it + is used instead of installing the ``grub2`` bootloader by Anaconda. + +:Links: + - https://github.com/rhinstaller/anaconda/pull/5342