diff --git a/coriolis/osmorphing/base.py b/coriolis/osmorphing/base.py index 9dcdf1ad..b13c0f2a 100644 --- a/coriolis/osmorphing/base.py +++ b/coriolis/osmorphing/base.py @@ -228,6 +228,7 @@ def __init__(self, conn, os_root_dir, os_root_dev, hypervisor, conn, os_root_dir, os_root_dev, hypervisor, event_manager, detected_os_info, osmorphing_parameters, operation_timeout) self._ssh = conn + self._grub2_update_scheduled = False @classmethod def get_required_detected_os_info_fields(cls): @@ -335,6 +336,7 @@ def pre_packages_install(self, package_names): self._copy_resolv_conf() def post_packages_install(self, package_names): + self._run_scheduled_grub2_update() self._restore_resolv_conf() def pre_packages_uninstall(self, package_names): @@ -517,7 +519,7 @@ def _ensure_cloud_init_not_disabled(self): if self._test_path(grub_conf_disabler): self._exec_cmd_chroot( "sed -i '/cloud-init=disabled/d' %s" % grub_conf_disabler) - self._execute_update_grub() + self._schedule_grub2_update() def _reset_cloud_init_run(self): self._exec_cmd_chroot("cloud-init clean --logs") @@ -707,6 +709,25 @@ def _execute_update_grub(self): update_cmd = self.get_update_grub2_command() self._exec_cmd_chroot(update_cmd) + def _schedule_grub2_update(self): + """Flags that the GRUB2 config needs to be regenerated. + + This is used to avoid running the potentially slow update command + multiple times during OSMorphing. Instead, the update is deferred and + run once at the end of the process, in 'post_packages_install'. + """ + self._grub2_update_scheduled = True + + def _run_scheduled_grub2_update(self): + """Runs the deferred GRUB2 config regeneration, if one was scheduled. + + The pending flag is cleared so that a subsequent call is a no-op + unless another GRUB modification schedules an update again. + """ + if self._grub2_update_scheduled: + self._execute_update_grub() + self._grub2_update_scheduled = False + def _apply_grub2_config(self, config_obj, execute_update_grub=True): self._validate_grub_config_obj(config_obj) @@ -714,7 +735,7 @@ def _apply_grub2_config(self, config_obj, "mv -f %s %s" % ( config_obj["location"], config_obj["source"])) if execute_update_grub: - self._execute_update_grub() + self._schedule_grub2_update() def _set_grub2_console_settings(self, consoles=None, speed=None, parity=None, grub_conf=None, diff --git a/coriolis/osmorphing/debian.py b/coriolis/osmorphing/debian.py index 598599ef..440cf934 100644 --- a/coriolis/osmorphing/debian.py +++ b/coriolis/osmorphing/debian.py @@ -60,7 +60,7 @@ def disable_predictable_nic_names(self): "GRUB_CMDLINE_LINUX", {"opt_type": "key_val", "opt_key": "biosdevname", "opt_val": 0}) self._write_file_sudo("etc/default/grub", cfg.dump()) - self._exec_cmd_chroot("/usr/sbin/update-grub") + self._schedule_grub2_update() def get_update_grub2_command(self): return "update-grub" @@ -134,7 +134,7 @@ def _install_uefi_fallback_bootloader(self): f"grub-install --removable --target={arch}-efi " "--efi-directory=/boot/efi --uefi-secure-boot" ) - self._exec_cmd_chroot("update-grub") + self._schedule_grub2_update() def set_net_config(self, nics_info, dhcp): if not dhcp: diff --git a/coriolis/tests/osmorphing/test_base.py b/coriolis/tests/osmorphing/test_base.py index 2d121a7b..78c3472d 100644 --- a/coriolis/tests/osmorphing/test_base.py +++ b/coriolis/tests/osmorphing/test_base.py @@ -262,10 +262,14 @@ def test_pre_packages_install(self, mock_copy_resolv_conf): self.os_morphing_tools.pre_packages_install(mock.sentinel.package_name) mock_copy_resolv_conf.assert_called_once_with() + @mock.patch.object( + base.BaseLinuxOSMorphingTools, '_run_scheduled_grub2_update') @mock.patch.object(base.BaseLinuxOSMorphingTools, '_restore_resolv_conf') - def test_post_packages_install(self, mock_restore_resolv_conf): + def test_post_packages_install(self, mock_restore_resolv_conf, + mock_run_scheduled_grub2_update): self.os_morphing_tools.post_packages_install( mock.sentinel.package_name) + mock_run_scheduled_grub2_update.assert_called_once_with() mock_restore_resolv_conf.assert_called_once_with() @mock.patch.object(base.BaseLinuxOSMorphingTools, '_copy_resolv_conf') @@ -636,12 +640,13 @@ def test__disable_installer_cloud_config_no_file( ], True) ) @ddt.unpack - @mock.patch.object(base.BaseLinuxOSMorphingTools, "_execute_update_grub") + @mock.patch.object(base.BaseLinuxOSMorphingTools, "_schedule_grub2_update") @mock.patch.object(base.BaseLinuxOSMorphingTools, "_exec_cmd_chroot") @mock.patch.object(base.BaseLinuxOSMorphingTools, "_test_path") def test__ensure_cloud_init_not_disabled( self, test_path_results, expected_cmds, updates_grub, - mock__test_path, mock__exec_cmd_chroot, mock__execute_update_grub): + mock__test_path, mock__exec_cmd_chroot, + mock__schedule_grub2_update): mock__test_path.side_effect = test_path_results self.os_morphing_tools._ensure_cloud_init_not_disabled() @@ -650,9 +655,9 @@ def test__ensure_cloud_init_not_disabled( call.args[0] for call in mock__exec_cmd_chroot.call_args_list] self.assertEqual(called_cmds, expected_cmds) if updates_grub: - mock__execute_update_grub.assert_called_once() + mock__schedule_grub2_update.assert_called_once() else: - mock__execute_update_grub.assset_not_called() + mock__schedule_grub2_update.assert_not_called() @mock.patch.object(base.BaseLinuxOSMorphingTools, "_exec_cmd_chroot") def test__reset_cloud_init_run(self, mock__exec_cmd_chroot): @@ -1066,9 +1071,44 @@ def test__execute_update_grub(self, mock_get_update_grub2_command, ) @mock.patch.object(base.BaseLinuxOSMorphingTools, '_execute_update_grub') + def test__schedule_grub2_update(self, mock_execute_update_grub): + self.assertFalse(self.os_morphing_tools._grub2_update_scheduled) + + self.os_morphing_tools._schedule_grub2_update() + + self.assertTrue(self.os_morphing_tools._grub2_update_scheduled) + # Scheduling must not run the (slow) update command itself. + mock_execute_update_grub.assert_not_called() + + @mock.patch.object(base.BaseLinuxOSMorphingTools, '_execute_update_grub') + def test__run_scheduled_grub2_update_when_scheduled( + self, mock_execute_update_grub): + self.os_morphing_tools._grub2_update_scheduled = True + + self.os_morphing_tools._run_scheduled_grub2_update() + + mock_execute_update_grub.assert_called_once_with() + # The pending flag must be cleared so subsequent calls are no-ops. + self.assertFalse(self.os_morphing_tools._grub2_update_scheduled) + + # Running the update again must be a no-op. + self.os_morphing_tools._run_scheduled_grub2_update() + # Check that the update command was called only once. + self.assertEqual(1, mock_execute_update_grub.call_count) + + @mock.patch.object(base.BaseLinuxOSMorphingTools, '_execute_update_grub') + def test__run_scheduled_grub2_update_when_not_scheduled( + self, mock_execute_update_grub): + self.os_morphing_tools._grub2_update_scheduled = False + + self.os_morphing_tools._run_scheduled_grub2_update() + + mock_execute_update_grub.assert_not_called() + + @mock.patch.object(base.BaseLinuxOSMorphingTools, '_schedule_grub2_update') @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test__apply_grub2_config(self, mock_exec_cmd_chroot, - mock_execute_update_grub): + mock_schedule_grub2_update): config_obj = { 'location': mock.sentinel.location, 'source': mock.sentinel.source, @@ -1083,12 +1123,12 @@ def test__apply_grub2_config(self, mock_exec_cmd_chroot, mock_exec_cmd_chroot.assert_called_once_with( 'mv -f %s %s' % (config_obj['location'], config_obj['source']) ) - mock_execute_update_grub.assert_called_once_with() + mock_schedule_grub2_update.assert_called_once_with() - @mock.patch.object(base.BaseLinuxOSMorphingTools, '_execute_update_grub') + @mock.patch.object(base.BaseLinuxOSMorphingTools, '_schedule_grub2_update') @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test__apply_grub2_config_no_update_grub(self, mock_exec_cmd_chroot, - mock_execute_update_grub): + mock_schedule_grub2_update): config_obj = { 'location': mock.sentinel.location, 'source': mock.sentinel.source, @@ -1103,7 +1143,7 @@ def test__apply_grub2_config_no_update_grub(self, mock_exec_cmd_chroot, mock_exec_cmd_chroot.assert_called_once_with( 'mv -f %s %s' % (config_obj['location'], config_obj['source']) ) - mock_execute_update_grub.assert_not_called() + mock_schedule_grub2_update.assert_not_called() def test__set_grub2_console_settings_invalid_parity(self): self.assertRaises( diff --git a/coriolis/tests/osmorphing/test_debian.py b/coriolis/tests/osmorphing/test_debian.py index d3400112..722778e9 100644 --- a/coriolis/tests/osmorphing/test_debian.py +++ b/coriolis/tests/osmorphing/test_debian.py @@ -48,15 +48,16 @@ def test_check_os_not_supported(self): self.assertFalse(result) + @mock.patch.object( + debian.BaseDebianMorphingTools, '_schedule_grub2_update') @mock.patch('coriolis.utils.Grub2ConfigEditor') @mock.patch.object(debian.BaseDebianMorphingTools, '_test_path_chroot') - @mock.patch.object(debian.BaseDebianMorphingTools, '_exec_cmd_chroot') @mock.patch.object(debian.BaseDebianMorphingTools, '_write_file_sudo') @mock.patch.object(debian.BaseDebianMorphingTools, '_read_file_sudo') def test_disable_predictable_nic_names( self, mock_read_file_sudo, mock_write_file_sudo, - mock_exec_cmd_chroot, mock_test_path_chroot, - mock_grub2_cfg_editor): + mock_test_path_chroot, mock_grub2_cfg_editor, + mock_schedule_grub2_update): mock_test_path_chroot.return_value = True self.morpher.disable_predictable_nic_names() @@ -87,7 +88,7 @@ def test_disable_predictable_nic_names( mock_read_file_sudo.assert_called_once_with('etc/default/grub') mock_write_file_sudo.assert_called_once_with( "etc/default/grub", mock_grub2_cfg_editor.return_value.dump()) - mock_exec_cmd_chroot.assert_called_once_with("/usr/sbin/update-grub") + mock_schedule_grub2_update.assert_called_once_with() @mock.patch('coriolis.utils.Grub2ConfigEditor') @mock.patch.object(debian.BaseDebianMorphingTools, '_exec_cmd_chroot') @@ -430,20 +431,21 @@ def test_add_wheezy_backports_already_included( mock_write_file_sudo.assert_not_called() mock_exec_cmd_chroot.assert_not_called() + @mock.patch.object( + debian.BaseDebianMorphingTools, '_schedule_grub2_update') @mock.patch.object(debian.BaseDebianMorphingTools, '_test_path_chroot') @mock.patch.object(debian.BaseDebianMorphingTools, '_exec_cmd_chroot') def test_install_uefi_fallback_bootloader( self, mock_exec_cmd_chroot, mock_test_path_chroot, + mock_schedule_grub2_update, ): mock_exec_cmd_chroot.side_effect = [ # uname -m "x86_64", # grub-install "", - # update-grub - "", ] mock_test_path_chroot.return_value = False @@ -454,8 +456,9 @@ def test_install_uefi_fallback_bootloader( mock.call( "grub-install --removable --target=x86_64-efi " "--efi-directory=/boot/efi --uefi-secure-boot"), - mock.call("update-grub"), ]) + # The grub update must be deferred (scheduled) rather than run now. + mock_schedule_grub2_update.assert_called_once_with() mock_test_path_chroot.assert_called_once_with( "/boot/efi/EFI/BOOT/BOOTX64.efi")