From 853f58323f13101f34330eca1916b02aa78c0f12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Ravier?= Date: Wed, 27 Jul 2022 19:56:30 +0200 Subject: [PATCH 1/8] rpm-ostree: Setup readonly sysroot for ostree & rw karg - Enable read only sysroot in the ostree repo config. - Add `rw` to the kernel arguments to keep statefull parts of the system (/var & /etc) writable. - Update units tests to account for the new rw karg (cherry-picked from a commit 0e00c90882) Related: RHEL-2250 --- .../payloads/payload/rpm_ostree/installation.py | 15 ++++++++++++++- .../payloads/payload/test_rpm_ostree_tasks.py | 4 ++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py b/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py index d995a4d8353..e4a06721850 100644 --- a/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py +++ b/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py @@ -456,6 +456,8 @@ def _set_kargs(self): if root_data.type == "btrfs subvolume": set_kargs_args.append("rootflags=subvol=" + root_name) + set_kargs_args.append("rw") + safe_exec_with_redirect("ostree", set_kargs_args, root=self._sysroot) @@ -515,7 +517,18 @@ def run(self): self._data.remote + ':' + ref] ) - log.info("ostree deploy complete") + log.info("ostree config set sysroot.readonly true") + + safe_exec_with_redirect( + "ostree", + ["config", + "--repo=" + self._sysroot + "/ostree/repo", + "set", + "sysroot.readonly", + "true"] + ) + + log.info("ostree admin deploy complete") self.report_progress(_("Deployment complete: {}").format(ref)) 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 862f6c1f640..ec9b9926015 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 @@ -625,7 +625,7 @@ def test_btrfs_run(self, devdata_mock, storage_mock, symlink_mock, rename_mock, exec_mock.assert_called_once_with( "ostree", ["admin", "instutil", "set-kargs", "BOOTLOADER-ARGS", "root=FSTAB-SPEC", - "rootflags=subvol=device-name"], + "rootflags=subvol=device-name", "rw"], root=sysroot ) @@ -661,7 +661,7 @@ def test_nonbtrfs_run(self, devdata_mock, storage_mock, symlink_mock, rename_moc ) exec_mock.assert_called_once_with( "ostree", - ["admin", "instutil", "set-kargs", "BOOTLOADER-ARGS", "root=FSTAB-SPEC"], + ["admin", "instutil", "set-kargs", "BOOTLOADER-ARGS", "root=FSTAB-SPEC", "rw"], root=sysroot ) From 5f5fc5caabb91ff51aa38a77d18c6f2ce41e1b05 Mon Sep 17 00:00:00 2001 From: Vendula Poncova Date: Fri, 4 Jun 2021 17:11:15 +0200 Subject: [PATCH 2/8] Apply the bootloader options before the installation Some of the bootloader options are not needed until the installation. If we apply them during the scheduling of the partitions, it won't be possible to change them after that, for example, from an add-on. Let's apply these options later, right before the bootloader installation. (cherry-picked from a commit dbac59a) Related: RHEL-17205 --- pyanaconda/modules/storage/bootloader/base.py | 56 +++++++++++++++++-- .../modules/storage/bootloader/execution.py | 44 +-------------- .../modules/storage/bootloader/utils.py | 5 +- .../modules/storage/test_module_bootloader.py | 2 +- 4 files changed, 56 insertions(+), 51 deletions(-) diff --git a/pyanaconda/modules/storage/bootloader/base.py b/pyanaconda/modules/storage/bootloader/base.py index e2db7dd38c8..9573746c33e 100644 --- a/pyanaconda/modules/storage/bootloader/base.py +++ b/pyanaconda/modules/storage/bootloader/base.py @@ -25,6 +25,7 @@ from blivet.iscsi import iscsi from blivet.size import Size +from pyanaconda.core.constants import BOOTLOADER_TIMEOUT_UNSET from pyanaconda.modules.common.util import is_module_available from pyanaconda.network import iface_for_host_ip from pyanaconda.modules.storage.platform import platform, PLATFORM_DEVICE_TYPES, \ @@ -209,6 +210,8 @@ def __init__(self): # timeout in seconds self._timeout = None self.password = None + self.encrypted_password = None + self.secure = None # console/serial stuff self.console = "" @@ -720,17 +723,60 @@ def check(self): def timeout(self, seconds): self._timeout = seconds - def set_boot_args(self, storage): - """Set up the boot command line.""" - self._set_extra_boot_args() + def prepare(self, storage): + """Prepare the bootloader for the installation. + + FIXME: Move this function into a task. + """ + bootloader_proxy = STORAGE.get_proxy(BOOTLOADER) + self._update_flags(bootloader_proxy) + self._apply_password(bootloader_proxy) + self._apply_timeout(bootloader_proxy) + self._apply_zipl_secure_boot(bootloader_proxy) + self._set_extra_boot_args(bootloader_proxy) self._set_storage_boot_args(storage) self._preserve_some_boot_args() self._set_graphical_boot_args() self._set_security_boot_args() - def _set_extra_boot_args(self): + def _update_flags(self, bootloader_proxy): + """Update flags.""" + if bootloader_proxy.KeepMBR: + log.debug("Don't update the MBR.") + self.keep_mbr = True + + if bootloader_proxy.KeepBootOrder: + log.debug("Don't change the existing boot order.") + self.keep_boot_order = True + + def _apply_password(self, bootloader_proxy): + """Set the password.""" + if bootloader_proxy.IsPasswordSet: + log.debug("Applying bootloader password.") + + if bootloader_proxy.IsPasswordEncrypted: + self.encrypted_password = bootloader_proxy.Password + else: + self.password = bootloader_proxy.Password + + def _apply_timeout(self, bootloader_proxy): + """Set the timeout.""" + timeout = bootloader_proxy.Timeout + if timeout != BOOTLOADER_TIMEOUT_UNSET: + log.debug("Applying bootloader timeout: %s", timeout) + self.timeout = timeout + + def _apply_zipl_secure_boot(self, bootloader_proxy): + """Set up the ZIPL Secure Boot.""" + if not blivet.arch.is_s390(): + return + + secure_boot = bootloader_proxy.ZIPLSecureBoot + log.debug("Applying ZIPL Secure Boot: %s", secure_boot) + self.secure = secure_boot + + def _set_extra_boot_args(self, bootloader_proxy): """Set the extra boot args.""" - bootloader_proxy = STORAGE.get_proxy(BOOTLOADER) self.boot_args.update(bootloader_proxy.ExtraArguments) def _set_storage_boot_args(self, storage): diff --git a/pyanaconda/modules/storage/bootloader/execution.py b/pyanaconda/modules/storage/bootloader/execution.py index b1dc4c64ccf..8f19c6085f6 100644 --- a/pyanaconda/modules/storage/bootloader/execution.py +++ b/pyanaconda/modules/storage/bootloader/execution.py @@ -23,7 +23,7 @@ from pyanaconda.core.configuration.anaconda import conf from pyanaconda.core.constants import BOOTLOADER_ENABLED, BOOTLOADER_SKIPPED, \ - BOOTLOADER_LOCATION_PARTITION, BOOTLOADER_TIMEOUT_UNSET + BOOTLOADER_LOCATION_PARTITION from pyanaconda.core.i18n import _ from pyanaconda.modules.common.constants.objects import BOOTLOADER from pyanaconda.modules.common.constants.services import STORAGE @@ -68,12 +68,8 @@ def execute(self, storage, dry_run=False): # Update the disk list. Disks are already sorted by Blivet. storage.bootloader.set_disk_list([d for d in storage.disks if d.partitioned]) - # Apply the settings. - self._update_flags(storage, bootloader_proxy) + # Apply settings related to boot devices. self._apply_location(storage, bootloader_proxy) - self._apply_password(storage, bootloader_proxy) - self._apply_timeout(storage, bootloader_proxy) - self._apply_zipl_secure_boot(storage, bootloader_proxy) self._apply_drive_order(storage, bootloader_proxy, dry_run=dry_run) self._apply_boot_drive(storage, bootloader_proxy, dry_run=dry_run) @@ -82,16 +78,6 @@ def execute(self, storage, dry_run=False): storage.bootloader.stage2_device = storage.boot_device storage.bootloader.set_stage1_device(storage.devices) - def _update_flags(self, storage, bootloader_proxy): - """Update flags.""" - if bootloader_proxy.KeepMBR: - log.debug("Don't update the MBR.") - storage.bootloader.keep_mbr = True - - if bootloader_proxy.KeepBootOrder: - log.debug("Don't change the existing boot order.") - storage.bootloader.keep_boot_order = True - def _apply_location(self, storage, bootloader_proxy): """Set the location.""" location = bootloader_proxy.PreferredLocation @@ -101,32 +87,6 @@ def _apply_location(self, storage, bootloader_proxy): "boot" if location == BOOTLOADER_LOCATION_PARTITION else "mbr" ) - def _apply_password(self, storage, bootloader_proxy): - """Set the password.""" - if bootloader_proxy.IsPasswordSet: - log.debug("Applying bootloader password.") - - if bootloader_proxy.IsPasswordEncrypted: - storage.bootloader.encrypted_password = bootloader_proxy.Password - else: - storage.bootloader.password = bootloader_proxy.Password - - def _apply_timeout(self, storage, bootloader_proxy): - """Set the timeout.""" - timeout = bootloader_proxy.Timeout - if timeout != BOOTLOADER_TIMEOUT_UNSET: - log.debug("Applying bootloader timeout: %s", timeout) - storage.bootloader.timeout = timeout - - def _apply_zipl_secure_boot(self, storage, bootloader_proxy): - """Set up the ZIPL Secure Boot.""" - if not blivet.arch.is_s390(): - return - - secure_boot = bootloader_proxy.ZIPLSecureBoot - log.debug("Applying ZIPL Secure Boot: %s", secure_boot) - storage.bootloader.secure = secure_boot - def _is_usable_disk(self, d): """Is the disk usable for the bootloader? diff --git a/pyanaconda/modules/storage/bootloader/utils.py b/pyanaconda/modules/storage/bootloader/utils.py index bd6641c74ff..faeb6b59c1e 100644 --- a/pyanaconda/modules/storage/bootloader/utils.py +++ b/pyanaconda/modules/storage/bootloader/utils.py @@ -208,9 +208,8 @@ def install_boot_loader(storage): stage2_device = storage.bootloader.stage2_device log.info("boot loader stage2 target device is %s", stage2_device.name) - # Set up the arguments. - # FIXME: do this from elsewhere? - storage.bootloader.set_boot_args(storage) + # Prepare the bootloader for the installation. + storage.bootloader.prepare(storage) # Install the bootloader. storage.bootloader.write() 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 8bf4e1075e9..bb1d53025ad 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 @@ -387,7 +387,7 @@ def test_install(self): bootloader.write.assert_not_called() InstallBootloaderTask(storage, BootloaderMode.ENABLED).run() - bootloader.set_boot_args.assert_called_once() + bootloader.prepare.assert_called_once() bootloader.write.assert_called_once() @patch('pyanaconda.modules.storage.bootloader.utils.execWithRedirect') From 3fe964e5ff82d762e543173d311da6cb3ec29e75 Mon Sep 17 00:00:00 2001 From: Vendula Poncova Date: Fri, 24 Nov 2023 17:13:33 +0100 Subject: [PATCH 3/8] bootloader: Remove the install_boot_loader function Move the implementation to the `InstallBootloaderTask` class. (cherry-picked from a commit e96bded) Related: RHEL-17205 --- .../storage/bootloader/installation.py | 26 +++++++++++++++++-- .../modules/storage/bootloader/utils.py | 24 +---------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/pyanaconda/modules/storage/bootloader/installation.py b/pyanaconda/modules/storage/bootloader/installation.py index 352d86ab85f..68a86c97b27 100644 --- a/pyanaconda/modules/storage/bootloader/installation.py +++ b/pyanaconda/modules/storage/bootloader/installation.py @@ -28,7 +28,7 @@ from pyanaconda.anaconda_loggers import get_module_logger from pyanaconda.modules.storage.bootloader.utils import configure_boot_loader, \ - install_boot_loader, recreate_initrds, create_rescue_images, create_bls_entries + recreate_initrds, create_rescue_images, create_bls_entries from pyanaconda.core.configuration.anaconda import conf from pyanaconda.modules.common.task import Task @@ -116,8 +116,14 @@ def __init__(self, storage, mode): @property def name(self): + """Name of the task.""" return "Install the bootloader" + @property + def _bootloader(self): + """Representation of the bootloader.""" + return self._storage.bootloader + def run(self): """Run the task. @@ -136,11 +142,27 @@ def run(self): return try: - install_boot_loader(storage=self._storage) + self._install_boot_loader() except BootLoaderError as e: log.exception("Bootloader installation has failed: %s", e) raise BootloaderInstallationError(str(e)) from None + def _install_boot_loader(self): + """Do the final write of the bootloader.""" + log.debug("Installing the boot loader.") + + stage1_device = self._bootloader.stage1_device + log.info("boot loader stage1 target device is %s", stage1_device.name) + + stage2_device = self._bootloader.stage2_device + log.info("boot loader stage2 target device is %s", stage2_device.name) + + # Prepare the bootloader for the installation. + self._bootloader.prepare(self._storage) + + # Install the bootloader. + self._bootloader.write() + class CreateBLSEntriesTask(Task): """The installation task that creates BLS entries.""" diff --git a/pyanaconda/modules/storage/bootloader/utils.py b/pyanaconda/modules/storage/bootloader/utils.py index faeb6b59c1e..c4370c5236c 100644 --- a/pyanaconda/modules/storage/bootloader/utils.py +++ b/pyanaconda/modules/storage/bootloader/utils.py @@ -27,8 +27,7 @@ from pyanaconda.anaconda_loggers import get_module_logger log = get_module_logger(__name__) -__all__ = ["configure_boot_loader", "install_boot_loader", "recreate_initrds", - "create_rescue_images"] +__all__ = ["configure_boot_loader", "recreate_initrds", "create_rescue_images"] def create_rescue_images(sysroot, kernel_versions): @@ -194,27 +193,6 @@ def _write_sysconfig_kernel(sysroot, storage): f.close() -def install_boot_loader(storage): - """Do the final write of the boot loader. - - :param storage: an instance of the storage - :raise: BootLoaderError if the installation fails - """ - log.debug("Installing the boot loader.") - - stage1_device = storage.bootloader.stage1_device - log.info("boot loader stage1 target device is %s", stage1_device.name) - - stage2_device = storage.bootloader.stage2_device - log.info("boot loader stage2 target device is %s", stage2_device.name) - - # Prepare the bootloader for the installation. - storage.bootloader.prepare(storage) - - # Install the bootloader. - storage.bootloader.write() - - def create_bls_entries(sysroot, storage, kernel_versions): """Create BLS entries. From 766f5d33e56cb9331ee0ef6a5d0614c4f9554cb6 Mon Sep 17 00:00:00 2001 From: Vendula Poncova Date: Fri, 24 Nov 2023 18:20:45 +0100 Subject: [PATCH 4/8] bootloader: Add the collect_arguments method Add a new method for collecting kernel arguments for the installation. (cherry-picked from a commit 6967994) Related: RHEL-17205 --- pyanaconda/modules/storage/bootloader/base.py | 26 +++++++++++-------- .../storage/bootloader/installation.py | 16 +++++++----- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/pyanaconda/modules/storage/bootloader/base.py b/pyanaconda/modules/storage/bootloader/base.py index 9573746c33e..8d98d289c08 100644 --- a/pyanaconda/modules/storage/bootloader/base.py +++ b/pyanaconda/modules/storage/bootloader/base.py @@ -723,21 +723,13 @@ def check(self): def timeout(self, seconds): self._timeout = seconds - def prepare(self, storage): - """Prepare the bootloader for the installation. - - FIXME: Move this function into a task. - """ + def prepare(self): + """Prepare the bootloader for the installation.""" bootloader_proxy = STORAGE.get_proxy(BOOTLOADER) self._update_flags(bootloader_proxy) self._apply_password(bootloader_proxy) self._apply_timeout(bootloader_proxy) self._apply_zipl_secure_boot(bootloader_proxy) - self._set_extra_boot_args(bootloader_proxy) - self._set_storage_boot_args(storage) - self._preserve_some_boot_args() - self._set_graphical_boot_args() - self._set_security_boot_args() def _update_flags(self, bootloader_proxy): """Update flags.""" @@ -775,8 +767,20 @@ def _apply_zipl_secure_boot(self, bootloader_proxy): log.debug("Applying ZIPL Secure Boot: %s", secure_boot) self.secure = secure_boot - def _set_extra_boot_args(self, bootloader_proxy): + def collect_arguments(self, storage): + """Collect kernel arguments for the installation. + + FIXME: Move this code out of this class. + """ + self._set_extra_boot_args() + self._set_storage_boot_args(storage) + self._preserve_some_boot_args() + self._set_graphical_boot_args() + self._set_security_boot_args() + + def _set_extra_boot_args(self): """Set the extra boot args.""" + bootloader_proxy = STORAGE.get_proxy(BOOTLOADER) self.boot_args.update(bootloader_proxy.ExtraArguments) def _set_storage_boot_args(self, storage): diff --git a/pyanaconda/modules/storage/bootloader/installation.py b/pyanaconda/modules/storage/bootloader/installation.py index 68a86c97b27..15df25d3b16 100644 --- a/pyanaconda/modules/storage/bootloader/installation.py +++ b/pyanaconda/modules/storage/bootloader/installation.py @@ -142,14 +142,15 @@ def run(self): return try: + self._collect_kernel_arguments() self._install_boot_loader() except BootLoaderError as e: log.exception("Bootloader installation has failed: %s", e) raise BootloaderInstallationError(str(e)) from None - def _install_boot_loader(self): - """Do the final write of the bootloader.""" - log.debug("Installing the boot loader.") + def _collect_kernel_arguments(self): + """Collect kernel arguments.""" + log.debug("Collecting the kernel arguments.") stage1_device = self._bootloader.stage1_device log.info("boot loader stage1 target device is %s", stage1_device.name) @@ -157,10 +158,13 @@ def _install_boot_loader(self): stage2_device = self._bootloader.stage2_device log.info("boot loader stage2 target device is %s", stage2_device.name) - # Prepare the bootloader for the installation. - self._bootloader.prepare(self._storage) + self._bootloader.collect_arguments(self._storage) + + def _install_boot_loader(self): + """Do the final write of the bootloader.""" + log.debug("Installing the boot loader.") - # Install the bootloader. + self._bootloader.prepare() self._bootloader.write() From 1377c44f4a1a9edabf7c9763822e71ab979a1620 Mon Sep 17 00:00:00 2001 From: Vendula Poncova Date: Fri, 24 Nov 2023 18:37:56 +0100 Subject: [PATCH 5/8] bootloader: Create an installation task for collecting kernel arguments Use the `CollectKernelArgumentsTask` task to collect kernel arguments for the installation. It will be used by the bootupd support that needs to be able to collect kernel arguments without installing the bootloader. (cherry-picked from a commit 8a7641e) Related: RHEL-17205 --- .../modules/storage/bootloader/bootloader.py | 6 +- .../storage/bootloader/installation.py | 69 +++++++++++++------ .../modules/storage/test_module_bootloader.py | 17 ++++- 3 files changed, 69 insertions(+), 23 deletions(-) diff --git a/pyanaconda/modules/storage/bootloader/bootloader.py b/pyanaconda/modules/storage/bootloader/bootloader.py index c805c0d7853..2ca6f0778fa 100644 --- a/pyanaconda/modules/storage/bootloader/bootloader.py +++ b/pyanaconda/modules/storage/bootloader/bootloader.py @@ -38,7 +38,7 @@ from pyanaconda.modules.storage.bootloader.bootloader_interface import BootloaderInterface from pyanaconda.modules.storage.bootloader.installation import ConfigureBootloaderTask, \ InstallBootloaderTask, FixZIPLBootloaderTask, FixBTRFSBootloaderTask, RecreateInitrdsTask, \ - CreateRescueImagesTask, CreateBLSEntriesTask + CreateRescueImagesTask, CreateBLSEntriesTask, CollectKernelArgumentsTask from pyanaconda.modules.storage.constants import BootloaderMode, ZIPLSecureBoot log = get_module_logger(__name__) @@ -480,6 +480,10 @@ def install_bootloader_with_tasks(self, payload_type, kernel_versions): kernel_versions=kernel_versions, sysroot=conf.target.system_root ), + CollectKernelArgumentsTask( + storage=self.storage, + mode=self.bootloader_mode + ), InstallBootloaderTask( storage=self.storage, mode=self.bootloader_mode diff --git a/pyanaconda/modules/storage/bootloader/installation.py b/pyanaconda/modules/storage/bootloader/installation.py index 15df25d3b16..f058b904ca4 100644 --- a/pyanaconda/modules/storage/bootloader/installation.py +++ b/pyanaconda/modules/storage/bootloader/installation.py @@ -37,7 +37,7 @@ __all__ = ["ConfigureBootloaderTask", "InstallBootloaderTask", "FixBTRFSBootloaderTask", "FixZIPLBootloaderTask", "RecreateInitrdsTask", "CreateRescueImagesTask", - "CreateBLSEntriesTask"] + "CreateBLSEntriesTask", "CollectKernelArgumentsTask"] class CreateRescueImagesTask(Task): @@ -105,8 +105,8 @@ def run(self): ) -class InstallBootloaderTask(Task): - """Installation task for the bootloader.""" +class CollectKernelArgumentsTask(Task): + """Installation task for collecting the kernel arguments.""" def __init__(self, storage, mode): """Create a new task.""" @@ -117,7 +117,7 @@ def __init__(self, storage, mode): @property def name(self): """Name of the task.""" - return "Install the bootloader" + return "Collect kernel arguments" @property def _bootloader(self): @@ -125,10 +125,7 @@ def _bootloader(self): return self._storage.bootloader def run(self): - """Run the task. - - :raise: BootloaderInstallationError if the installation fails - """ + """Run the task.""" if conf.target.is_directory: log.debug("The bootloader installation is disabled for dir installations.") return @@ -141,15 +138,6 @@ def run(self): log.debug("The bootloader installation is skipped.") return - try: - self._collect_kernel_arguments() - self._install_boot_loader() - except BootLoaderError as e: - log.exception("Bootloader installation has failed: %s", e) - raise BootloaderInstallationError(str(e)) from None - - def _collect_kernel_arguments(self): - """Collect kernel arguments.""" log.debug("Collecting the kernel arguments.") stage1_device = self._bootloader.stage1_device @@ -160,12 +148,51 @@ def _collect_kernel_arguments(self): self._bootloader.collect_arguments(self._storage) - def _install_boot_loader(self): - """Do the final write of the bootloader.""" + +class InstallBootloaderTask(Task): + """Installation task for the bootloader.""" + + def __init__(self, storage, mode): + """Create a new task.""" + super().__init__() + self._storage = storage + self._mode = mode + + @property + def name(self): + """Name of the task.""" + return "Install the bootloader" + + @property + def _bootloader(self): + """Representation of the bootloader.""" + return self._storage.bootloader + + def run(self): + """Run the task. + + :raise: BootloaderInstallationError if the installation fails + """ + if conf.target.is_directory: + log.debug("The bootloader installation is disabled for dir installations.") + return + + if self._mode == BootloaderMode.DISABLED: + log.debug("The bootloader installation is disabled.") + return + + if self._mode == BootloaderMode.SKIPPED: + log.debug("The bootloader installation is skipped.") + return + log.debug("Installing the boot loader.") - self._bootloader.prepare() - self._bootloader.write() + try: + self._bootloader.prepare() + self._bootloader.write() + except BootLoaderError as e: + log.exception("Bootloader installation has failed: %s", e) + raise BootloaderInstallationError(str(e)) from None class CreateBLSEntriesTask(Task): 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 bb1d53025ad..2acde48eebc 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 @@ -51,7 +51,7 @@ from pyanaconda.modules.storage.bootloader.bootloader_interface import BootloaderInterface from pyanaconda.modules.storage.bootloader.installation import ConfigureBootloaderTask, \ InstallBootloaderTask, FixZIPLBootloaderTask, FixBTRFSBootloaderTask, RecreateInitrdsTask, \ - CreateRescueImagesTask, CreateBLSEntriesTask + CreateRescueImagesTask, CreateBLSEntriesTask, CollectKernelArgumentsTask class BootloaderInterfaceTestCase(unittest.TestCase): @@ -201,6 +201,7 @@ def test_install_bootloader_with_tasks(self, publisher): task_classes = [ CreateRescueImagesTask, ConfigureBootloaderTask, + CollectKernelArgumentsTask, InstallBootloaderTask, CreateBLSEntriesTask ] @@ -375,6 +376,20 @@ def test_configure(self): assert image.label == "anaconda" assert image.device == storage.root_device + def test_collect_kernel_arguments(self): + """Test the collection of the kernel arguments for the installation.""" + bootloader = Mock() + storage = Mock(bootloader=bootloader) + + CollectKernelArgumentsTask(storage, BootloaderMode.DISABLED).run() + bootloader.collect_arguments.assert_not_called() + + CollectKernelArgumentsTask(storage, BootloaderMode.SKIPPED).run() + bootloader.collect_arguments.assert_not_called() + + CollectKernelArgumentsTask(storage, BootloaderMode.ENABLED).run() + bootloader.collect_arguments.assert_called_once_with(storage) + def test_install(self): """Test the installation task for the boot loader.""" bootloader = Mock() From a42e2a078317a8ef8104ecb246c3097d3c36cfa8 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 6/8] 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. (cherry-picked from a commit 8e690d5) Resolves: RHEL-17205 --- .../payload/rpm_ostree/installation.py | 30 ++++++++++++- .../payloads/payload/rpm_ostree/util.py | 27 ++++++++++++ .../payloads/payload/test_rpm_ostree_tasks.py | 42 ++++++++++++++++++- .../payloads/payload/test_rpm_ostree_util.py | 35 ++++++++++++++++ 4 files changed, 132 insertions(+), 2 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 e4a06721850..cfc164e791c 100644 --- a/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py +++ b/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py @@ -24,10 +24,12 @@ from pyanaconda.core.glib import format_size_full, create_new_context, Variant, GError from pyanaconda.core.i18n import _ from pyanaconda.core.util import execWithRedirect, mkdirChain, set_system_root +from pyanaconda.modules.common.errors.installation import 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") @@ -417,9 +419,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..6cf1138414b --- /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.util 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 ec9b9926015..4861f32cb13 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 - def _make_config_data(): """Create OSTree configuration data for testing @@ -665,6 +664,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, is_dirinstall=False) + 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..c9a106084e2 --- /dev/null +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_rpm_ostree_util.py @@ -0,0 +1,35 @@ +# +# 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.util import join_paths, touch, mkdirChain as 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 bfe39f8bde8aea98756397add0238cb809e8de21 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 7/8] bootloader: Detect bootupd and skip regular install (cherry-picked from a commit 0d42d2f) Related: RHEL-17205 --- .../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 2ca6f0778fa..4ec52f0d46e 100644 --- a/pyanaconda/modules/storage/bootloader/bootloader.py +++ b/pyanaconda/modules/storage/bootloader/bootloader.py @@ -486,7 +486,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 f058b904ca4..74d61baba75 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.core.util import execInSysroot @@ -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: @@ -302,7 +309,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 2acde48eebc..1f91a1771ec 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.util import mkdirChain as 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): @@ -664,7 +705,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 a658b4bf3fedf27bc5a51a95d815c0a876b8a1e4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 6 Dec 2023 10:58:56 -0500 Subject: [PATCH 8/8] bootupd: Use --write-uuid This is an even newer behavior that takes over handling of the "UUID stamp files", which we want in general instead of using the static labels. Note `--write-uuid` implies `--with-static-configs`. This should fix this use case: ``` clearpart --all --initlabel --disklabel=gpt reqpart --add-boot part / --grow --fstype xfs ``` Whereas right now we require: ``` clearpart --all --initlabel --disklabel=gpt reqpart part /boot --size=1000 --fstype=ext4 --label=boot part / --grow --fstype xfs ``` Specifically the `--label=boot`. (cherry-picked from a commit 7b091de) Related: RHEL-17205 --- pyanaconda/modules/payloads/payload/rpm_ostree/installation.py | 2 +- .../modules/payloads/payload/test_rpm_ostree_tasks.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py b/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py index cfc164e791c..a03b8959ba6 100644 --- a/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py +++ b/pyanaconda/modules/payloads/payload/rpm_ostree/installation.py @@ -436,7 +436,7 @@ def _install_bootupd(self): "backend", "install", "--auto", - "--with-static-configs", + "--write-uuid", "--device", dev_data.path, "/", 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 4861f32cb13..b32fff05b31 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 @@ -694,7 +694,7 @@ def test_bootupd_run(self, devdata_mock, storage_mock, symlink_mock, rename_mock exec_mock.assert_has_calls([ call( "bootupctl", - ["backend", "install", "--auto", "--with-static-configs", "--device", + ["backend", "install", "--auto", "--write-uuid", "--device", "/dev/btldr-drv", "/"], root=sysroot ),