Add post-hook functionality when finalizing osmorphing

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,14 +709,33 @@ 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)
         self._exec_cmd_chroot(
             "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,

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:

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(

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")