From f734c549c7330c4569eccf61ad6e1a1c258ef325 Mon Sep 17 00:00:00 2001 From: Shivanand Tendulker Date: Wed, 2 Sep 2015 00:59:14 -0700 Subject: [PATCH] Set boot device in PXE Boot interface method prepare_instance() Boot device order may get lost if boot mode changes during deploy. This fix will set appropriate boot device even if boot device order was lost. Closes-Bug: 1491488 Change-Id: I2f7d16b0c7755a2a7ff9afc1c00ed93b5e6f0871 --- ironic/drivers/modules/iscsi_deploy.py | 3 --- ironic/drivers/modules/pxe.py | 9 ++++++++- ironic/tests/drivers/test_iscsi_deploy.py | 16 +--------------- ironic/tests/drivers/test_pxe.py | 18 +++++++++++++++--- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 20f5d0adb2..69b6ac0256 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -21,7 +21,6 @@ from oslo_utils import fileutils from oslo_utils import strutils from six.moves.urllib import parse -from ironic.common import boot_devices from ironic.common import exception from ironic.common.glance_service import service_utils as glance_service_utils from ironic.common.i18n import _ @@ -838,8 +837,6 @@ class VendorPassthru(agent_base_vendor.BaseAgentVendor): task.driver.boot.prepare_instance(task) if deploy_utils.get_boot_option(node) == "local": - deploy_utils.try_set_boot_device(task, boot_devices.DISK) - if not is_whole_disk_image: LOG.debug('Installing the bootloader on node %s', node.uuid) diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index b471f656fd..6ed1409267 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -574,7 +574,10 @@ class PXEBoot(base.BootInterface): pxe_config_path, root_uuid_or_disk_id, deploy_utils.get_boot_mode_for_deploy(node), iwdi, deploy_utils.is_trusted_boot_requested(node)) - + # In case boot mode changes from bios to uefi, boot device + # order may get lost in some platforms. Better to re-apply + # boot device. + deploy_utils.try_set_boot_device(task, boot_devices.PXE) else: # If it's going to boot from the local disk, we don't need # PXE config files. They still need to be generated as part @@ -582,6 +585,10 @@ class PXEBoot(base.BootInterface): # deploy ramdisk pxe_utils.clean_up_pxe_config(task) + # In case boot mode changes from bios to uefi, boot device order + # may get lost in some platforms. Better to re-apply boot device. + deploy_utils.try_set_boot_device(task, boot_devices.DISK) + def clean_up_instance(self, task): """Cleans up the boot of instance. diff --git a/ironic/tests/drivers/test_iscsi_deploy.py b/ironic/tests/drivers/test_iscsi_deploy.py index bd4b438274..f1c913393d 100644 --- a/ironic/tests/drivers/test_iscsi_deploy.py +++ b/ironic/tests/drivers/test_iscsi_deploy.py @@ -23,7 +23,6 @@ from oslo_config import cfg from oslo_utils import fileutils from oslo_utils import uuidutils -from ironic.common import boot_devices from ironic.common import driver_factory from ironic.common import exception from ironic.common import keystone @@ -1097,14 +1096,13 @@ class TestVendorPassthru(db_base.DbTestCase): key='fake-12345') @mock.patch.object(fake.FakeBoot, 'prepare_instance', autospec=True) - @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(deploy_utils, 'notify_ramdisk_to_proceed', autospec=True) @mock.patch.object(iscsi_deploy, 'InstanceImageCache', autospec=True) @mock.patch.object(deploy_utils, 'deploy_partition_image', autospec=True) def _test_pass_deploy_info_deploy(self, is_localboot, mock_deploy, mock_image_cache, - notify_mock, mock_node_boot_dev, + notify_mock, fakeboot_prepare_instance_mock): # set local boot i_info = self.node.instance_info @@ -1134,14 +1132,8 @@ class TestVendorPassthru(db_base.DbTestCase): mock_image_cache.return_value.clean_up.assert_called_once_with() notify_mock.assert_called_once_with('123456') fakeboot_prepare_instance_mock.assert_called_once_with(mock.ANY, task) - if is_localboot: - mock_node_boot_dev.assert_called_once_with( - mock.ANY, boot_devices.DISK, persistent=True) - else: - self.assertFalse(mock_node_boot_dev.called) @mock.patch.object(fake.FakeBoot, 'prepare_instance', autospec=True) - @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(deploy_utils, 'notify_ramdisk_to_proceed', autospec=True) @mock.patch.object(iscsi_deploy, 'InstanceImageCache', autospec=True) @@ -1150,7 +1142,6 @@ class TestVendorPassthru(db_base.DbTestCase): mock_deploy, mock_image_cache, notify_mock, - mock_node_boot_dev, fakeboot_prep_inst_mock): i_info = self.node.instance_info # set local boot @@ -1181,11 +1172,6 @@ class TestVendorPassthru(db_base.DbTestCase): mock_image_cache.return_value.clean_up.assert_called_once_with() notify_mock.assert_called_once_with('123456') fakeboot_prep_inst_mock.assert_called_once_with(mock.ANY, task) - if is_localboot: - mock_node_boot_dev.assert_called_once_with( - mock.ANY, boot_devices.DISK, persistent=True) - else: - self.assertFalse(mock_node_boot_dev.called) def test_pass_deploy_info_deploy(self): self._test_pass_deploy_info_deploy(False) diff --git a/ironic/tests/drivers/test_pxe.py b/ironic/tests/drivers/test_pxe.py index c3e296d46d..465cf5221f 100644 --- a/ironic/tests/drivers/test_pxe.py +++ b/ironic/tests/drivers/test_pxe.py @@ -24,6 +24,7 @@ from oslo_config import cfg from oslo_serialization import jsonutils as json from oslo_utils import fileutils +from ironic.common import boot_devices from ironic.common import dhcp_factory from ironic.common import exception from ironic.common.glance_service import base_image_service @@ -815,13 +816,15 @@ class PXEBootTestCase(db_base.DbTestCase): clean_up_pxe_env_mock.assert_called_once_with(task, image_info) get_deploy_image_info_mock.assert_called_once_with(task.node) + @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @mock.patch.object(deploy_utils, 'switch_pxe_config', autospec=True) @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) @mock.patch.object(pxe, '_cache_ramdisk_kernel', autospec=True) @mock.patch.object(pxe, '_get_instance_image_info', autospec=True) def test_prepare_instance_netboot( self, get_image_info_mock, cache_mock, - dhcp_factory_mock, switch_pxe_config_mock): + dhcp_factory_mock, switch_pxe_config_mock, + set_boot_device_mock): provider_mock = mock.MagicMock() dhcp_factory_mock.return_value = provider_mock image_info = {'kernel': ('', '/path/to/kernel'), @@ -846,14 +849,18 @@ class PXEBootTestCase(db_base.DbTestCase): switch_pxe_config_mock.assert_called_once_with( pxe_config_path, "30212642-09d3-467f-8e09-21685826ab50", 'bios', False, False) + set_boot_device_mock.assert_called_once_with(task, + boot_devices.PXE) + @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @mock.patch.object(deploy_utils, 'switch_pxe_config', autospec=True) @mock.patch.object(dhcp_factory, 'DHCPFactory') @mock.patch.object(pxe, '_cache_ramdisk_kernel', autospec=True) @mock.patch.object(pxe, '_get_instance_image_info', autospec=True) def test_prepare_instance_netboot_missing_root_uuid( self, get_image_info_mock, cache_mock, - dhcp_factory_mock, switch_pxe_config_mock): + dhcp_factory_mock, switch_pxe_config_mock, + set_boot_device_mock): provider_mock = mock.MagicMock() dhcp_factory_mock.return_value = provider_mock image_info = {'kernel': ('', '/path/to/kernel'), @@ -872,13 +879,18 @@ class PXEBootTestCase(db_base.DbTestCase): task.context, task.node, image_info) provider_mock.update_dhcp.assert_called_once_with(task, dhcp_opts) self.assertFalse(switch_pxe_config_mock.called) + self.assertFalse(set_boot_device_mock.called) + @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @mock.patch.object(pxe_utils, 'clean_up_pxe_config', autospec=True) - def test_prepare_instance_localboot(self, clean_up_pxe_config_mock): + def test_prepare_instance_localboot(self, clean_up_pxe_config_mock, + set_boot_device_mock): with task_manager.acquire(self.context, self.node.uuid) as task: task.node.instance_info['capabilities'] = {'boot_option': 'local'} task.driver.boot.prepare_instance(task) clean_up_pxe_config_mock.assert_called_once_with(task) + set_boot_device_mock.assert_called_once_with(task, + boot_devices.DISK) @mock.patch.object(pxe, '_clean_up_pxe_env', autospec=True) @mock.patch.object(pxe, '_get_instance_image_info', autospec=True)