diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index 3b624d07e4..542a1a459e 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -346,7 +346,8 @@ class BaseAgentVendor(base.VendorInterface): task.process_event('done') LOG.info(_LI('Deployment to node %s done'), task.node.uuid) - def configure_local_boot(self, task, root_uuid): + def configure_local_boot(self, task, root_uuid, + efi_system_part_uuid=None): """Helper method to configure local boot on the node. This method triggers bootloader installation on the node. @@ -356,12 +357,16 @@ class BaseAgentVendor(base.VendorInterface): :param task: a TaskManager object containing the node :param root_uuid: The UUID of the root partition. This is used for identifying the partition which contains the image deployed. + :param efi_system_part_uuid: The UUID of the efi system partition. + This is used only in uef boot mode. :raises: InstanceDeployFailure if bootloader installation failed or on encountering error while setting the boot device on the node. """ node = task.node if not node.driver_internal_info.get('is_whole_disk_image'): - result = self._client.install_bootloader(node, root_uuid) + result = self._client.install_bootloader( + node, root_uuid=root_uuid, + efi_system_part_uuid=efi_system_part_uuid) if result['command_status'] == 'FAILED': msg = (_("Failed to install a bootloader when " "deploying node %(node)s. Error: %(error)s") % diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py index 7125530f87..4bb2b49ada 100644 --- a/ironic/drivers/modules/agent_client.py +++ b/ironic/drivers/modules/agent_client.py @@ -106,9 +106,10 @@ class AgentClient(object): params=params, wait=True) - def install_bootloader(self, node, root_uuid): + def install_bootloader(self, node, root_uuid, efi_system_part_uuid=None): """Install a boot loader on the image.""" - params = {'root_uuid': root_uuid} + params = {'root_uuid': root_uuid, + 'efi_system_part_uuid': efi_system_part_uuid} return self._command(node=node, method='image.install_bootloader', params=params, diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 7eeb6c6f37..1c43aeffdb 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -557,7 +557,8 @@ def work_on_disk(dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, or configdrive HTTP URL. :param boot_option: Can be "local" or "netboot". "netboot" by default. :param boot_mode: Can be "bios" or "uefi". "bios" by default. - :returns: the UUID of the root partition. + :returns: a dictionary containing the UUID of root partition and efi system + partition (if boot mode is uefi). """ # the only way for preserve_ephemeral to be set to true is if we are # rebuilding an instance with --preserve_ephemeral. @@ -624,13 +625,21 @@ def work_on_disk(dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, if ephemeral_part and not preserve_ephemeral: mkfs(dev=ephemeral_part, fs=ephemeral_format, label="ephemeral0") + uuids_to_return = { + 'root uuid': root_part, + 'efi system partition uuid': part_dict.get('efi system partition') + } + try: - root_uuid = block_uuid(root_part) + for part, part_dev in six.iteritems(uuids_to_return): + if part_dev: + uuids_to_return[part] = block_uuid(part_dev) + except processutils.ProcessExecutionError: with excutils.save_and_reraise_exception(): - LOG.error(_LE("Failed to detect root device UUID.")) + LOG.error(_LE("Failed to detect %s"), part) - return root_uuid + return uuids_to_return def deploy_partition_image(address, port, iqn, lun, image_path, @@ -658,7 +667,8 @@ def deploy_partition_image(address, port, iqn, lun, image_path, or configdrive HTTP URL. :param boot_option: Can be "local" or "netboot". "netboot" by default. :param boot_mode: Can be "bios" or "uefi". "bios" by default. - :returns: the UUID of the root partition. + :returns: a dictionary containing the UUID of root partition and efi system + partition (if boot mode is uefi). """ with _iscsi_setup_and_handle_errors(address, port, iqn, lun, image_path) as dev: @@ -666,14 +676,13 @@ def deploy_partition_image(address, port, iqn, lun, image_path, if image_mb > root_mb: root_mb = image_mb - root_uuid = work_on_disk(dev, root_mb, swap_mb, ephemeral_mb, - ephemeral_format, image_path, node_uuid, - preserve_ephemeral=preserve_ephemeral, - configdrive=configdrive, - boot_option=boot_option, - boot_mode=boot_mode) + uuid_dict_returned = work_on_disk( + dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, image_path, + node_uuid, preserve_ephemeral=preserve_ephemeral, + configdrive=configdrive, boot_option=boot_option, + boot_mode=boot_mode) - return root_uuid + return uuid_dict_returned def deploy_disk_image(address, port, iqn, lun, @@ -687,13 +696,14 @@ def deploy_disk_image(address, port, iqn, lun, :param image_path: Path for the instance's disk image. :param node_uuid: node's uuid. Used for logging. Currently not in use by this function but could be used in the future. + :returns: a dictionary containing the disk identifier for the disk. """ with _iscsi_setup_and_handle_errors(address, port, iqn, lun, image_path) as dev: populate_image(image_path, dev) disk_identifier = get_disk_identifier(dev) - return disk_identifier + return {'disk identifier': disk_identifier} @contextlib.contextmanager diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index 8046f586bc..e5b39a8db3 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/ironic/drivers/modules/ilo/deploy.py @@ -567,8 +567,12 @@ class VendorPassthru(agent_base_vendor.BaseAgentVendor): iwdi = node.driver_internal_info.get('is_whole_disk_image') ilo_common.cleanup_vmedia_boot(task) - root_uuid_or_disk_id = iscsi_deploy.continue_deploy(task, **kwargs) + uuid_dict_returned = iscsi_deploy.continue_deploy(task, **kwargs) + root_uuid_or_disk_id = uuid_dict_returned.get( + 'root uuid', uuid_dict_returned.get('disk identifier')) + # TODO(rameshg87): It's not correct to return here as it will leave + # the node in DEPLOYING state. This will be fixed in bug 1405519. if not root_uuid_or_disk_id: return @@ -612,10 +616,16 @@ class VendorPassthru(agent_base_vendor.BaseAgentVendor): ilo_common.cleanup_vmedia_boot(task) - root_uuid = iscsi_deploy.do_agent_iscsi_deploy(task, self._client) + uuid_dict_returned = iscsi_deploy.do_agent_iscsi_deploy(task, + self._client) + root_uuid = uuid_dict_returned.get('root uuid') if iscsi_deploy.get_boot_option(node) == "local": - self.configure_local_boot(task, root_uuid) + efi_system_part_uuid = uuid_dict_returned.get( + 'efi system partition uuid') + self.configure_local_boot( + task, root_uuid=root_uuid, + efi_system_part_uuid=efi_system_part_uuid) else: # Agent vendorpassthru are made without auth token. # We require auth_token to talk to glance while building boot iso. diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 1a4a8fac38..cc0f301ad5 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -275,8 +275,11 @@ def continue_deploy(task, **kwargs): :param kwargs: the kwargs to be passed to deploy. :raises: InvalidState if the event is not allowed by the associated state machine. - :returns: UUID of the root partition for partition images or disk - identifier for whole disk images or None on error. + :returns: a dictionary containing some identifiers for the deployed + image. If it's partition image, then it returns root uuid and efi + system partition uuid (if boot mode is uefi). If it's whole disk + image, it returns disk identifier. On error cases, it returns an + empty dictionary. """ node = task.node @@ -288,7 +291,7 @@ def continue_deploy(task, **kwargs): ramdisk_error) deploy_utils.set_failed_state(task, _('Failure in deploy ramdisk.')) destroy_images(node.uuid) - return + return {} # NOTE(lucasagomes): Let's make sure we don't log the full content # of the config drive here because it can be up to 64MB in size, @@ -301,13 +304,12 @@ def continue_deploy(task, **kwargs): LOG.debug('Continuing deployment for node %(node)s, params %(params)s', {'node': node.uuid, 'params': log_params}) - root_uuid_or_disk_id = None + uuid_dict_returned = {} try: if node.driver_internal_info['is_whole_disk_image']: - root_uuid_or_disk_id = deploy_utils.deploy_disk_image(**params) + uuid_dict_returned = deploy_utils.deploy_disk_image(**params) else: - root_uuid_or_disk_id = deploy_utils.deploy_partition_image( - **params) + uuid_dict_returned = deploy_utils.deploy_partition_image(**params) except Exception as e: LOG.error(_LE('Deploy failed for instance %(instance)s. ' 'Error: %(error)s'), @@ -316,7 +318,7 @@ def continue_deploy(task, **kwargs): 'iSCSI deployment.')) destroy_images(node.uuid) - return root_uuid_or_disk_id + return uuid_dict_returned def do_agent_iscsi_deploy(task, agent_client): @@ -330,7 +332,10 @@ def do_agent_iscsi_deploy(task, agent_client): :param agent_client: an instance of agent_client.AgentClient which will be used during iscsi deploy (for exposing node's target disk via iSCSI, for install boot loader, etc). - :returns: UUID of the root partition which was deployed. + :returns: a dictionary containing some identifiers for the deployed + image. If it's partition image, then it returns root uuid and efi + system partition uuid (if boot mode is uefi). If it's whole disk + image, it returns disk identifier. :raises: InstanceDeployFailure, if it encounters some error during the deploy. """ @@ -359,7 +364,9 @@ def do_agent_iscsi_deploy(task, agent_client): 'key': iscsi_options['deployment_key'], 'address': address} - root_uuid_or_disk_id = continue_deploy(task, **iscsi_params) + uuid_dict_returned = continue_deploy(task, **iscsi_params) + root_uuid_or_disk_id = uuid_dict_returned.get( + 'root uuid', uuid_dict_returned.get('disk identifier')) if not root_uuid_or_disk_id: msg = (_("Couldn't determine the UUID of the root " "partition or the disk identifier when deploying " @@ -374,7 +381,7 @@ def do_agent_iscsi_deploy(task, agent_client): node.driver_internal_info = driver_internal_info node.save() - return root_uuid_or_disk_id + return uuid_dict_returned def get_boot_option(node): diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 99389fa29c..e90b6d4d3f 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -540,7 +540,12 @@ class VendorPassthru(agent_base_vendor.BaseAgentVendor): _destroy_token_file(node) is_whole_disk_image = node.driver_internal_info['is_whole_disk_image'] - root_uuid_or_disk_id = iscsi_deploy.continue_deploy(task, **kwargs) + uuid_dict_returned = iscsi_deploy.continue_deploy(task, **kwargs) + root_uuid_or_disk_id = uuid_dict_returned.get( + 'root uuid', uuid_dict_returned.get('disk identifier')) + + # TODO(rameshg87): It's not correct to return here as it will leave + # the node in DEPLOYING state. This will be fixed in bug 1405519. if not root_uuid_or_disk_id: return @@ -599,23 +604,28 @@ class VendorPassthru(agent_base_vendor.BaseAgentVendor): # it here. _destroy_token_file(node) - root_uuid_or_disk_id = iscsi_deploy.do_agent_iscsi_deploy( - task, - self._client) + uuid_dict_returned = iscsi_deploy.do_agent_iscsi_deploy(task, + self._client) + is_whole_disk_image = node.driver_internal_info['is_whole_disk_image'] if iscsi_deploy.get_boot_option(node) == "local": # Install the boot loader - self.configure_local_boot(task, root_uuid_or_disk_id) + root_uuid = uuid_dict_returned.get('root uuid') + efi_sys_uuid = uuid_dict_returned.get('efi system partition uuid') + self.configure_local_boot( + task, root_uuid=root_uuid, + efi_system_part_uuid=efi_sys_uuid) # If it's going to boot from the local disk, get rid of # the PXE configuration files used for the deployment pxe_utils.clean_up_pxe_config(task) else: + root_uuid_or_disk_id = uuid_dict_returned.get( + 'root uuid', uuid_dict_returned.get('disk identifier')) pxe_config_path = pxe_utils.get_pxe_config_file_path(node.uuid) boot_mode = driver_utils.get_node_capability(node, 'boot_mode') - deploy_utils.switch_pxe_config( - pxe_config_path, - root_uuid_or_disk_id, - boot_mode, is_whole_disk_image) + deploy_utils.switch_pxe_config(pxe_config_path, + root_uuid_or_disk_id, + boot_mode, is_whole_disk_image) self.reboot_and_finish_deploy(task) diff --git a/ironic/tests/drivers/ilo/test_deploy.py b/ironic/tests/drivers/ilo/test_deploy.py index 6b0743a627..6fb7b8d4b5 100644 --- a/ironic/tests/drivers/ilo/test_deploy.py +++ b/ironic/tests/drivers/ilo/test_deploy.py @@ -425,7 +425,7 @@ class VendorPassthruTestCase(db_base.DbTestCase): setup_vmedia_mock, set_boot_device_mock, notify_deploy_complete_mock): kwargs = {'method': 'pass_deploy_info', 'address': '123456'} - continue_deploy_mock.return_value = None + continue_deploy_mock.return_value = {} get_boot_iso_mock.return_value = 'boot-iso' self.node.provision_state = states.DEPLOYWAIT @@ -457,7 +457,7 @@ class VendorPassthruTestCase(db_base.DbTestCase): setup_vmedia_mock, set_boot_device_mock, notify_deploy_complete_mock): kwargs = {'method': 'pass_deploy_info', 'address': '123456'} - continue_deploy_mock.return_value = 'root-uuid' + continue_deploy_mock.return_value = {'root uuid': 'root-uuid'} get_boot_iso_mock.return_value = 'boot-iso' self.node.provision_state = states.DEPLOYWAIT @@ -503,7 +503,7 @@ class VendorPassthruTestCase(db_base.DbTestCase): def test__continue_deploy_no_root_uuid(self, cleanup_vmedia_boot_mock, continue_deploy_mock): kwargs = {'address': '123456'} - continue_deploy_mock.return_value = None + continue_deploy_mock.return_value = {} self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE @@ -524,7 +524,7 @@ class VendorPassthruTestCase(db_base.DbTestCase): def test__continue_deploy_create_boot_iso_fail(self, get_iso_mock, cleanup_vmedia_boot_mock, continue_deploy_mock, node_power_mock): kwargs = {'address': '123456'} - continue_deploy_mock.return_value = 'root-uuid' + continue_deploy_mock.return_value = {'root uuid': 'root-uuid'} get_iso_mock.side_effect = exception.ImageCreationFailed( image_type='iso', error="error") self.node.provision_state = states.DEPLOYWAIT @@ -553,7 +553,7 @@ class VendorPassthruTestCase(db_base.DbTestCase): notify_deploy_complete_mock): kwargs = {'method': 'pass_deploy_info', 'address': '123456'} - continue_deploy_mock.return_value = '' + continue_deploy_mock.return_value = {'root uuid': ''} self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE @@ -596,7 +596,8 @@ class VendorPassthruTestCase(db_base.DbTestCase): self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.DEPLOYING self.node.save() - do_agent_iscsi_deploy_mock.return_value = 'some-root-uuid' + do_agent_iscsi_deploy_mock.return_value = { + 'root uuid': 'some-root-uuid'} keystone_mock.return_value = 'admin-token' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: @@ -625,7 +626,8 @@ class VendorPassthruTestCase(db_base.DbTestCase): self.node.instance_info = { 'capabilities': {'boot_option': 'local'}} self.node.save() - do_agent_iscsi_deploy_mock.return_value = 'some-root-uuid' + do_agent_iscsi_deploy_mock.return_value = { + 'root uuid': 'some-root-uuid'} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.vendor.continue_deploy(task) @@ -633,7 +635,37 @@ class VendorPassthruTestCase(db_base.DbTestCase): do_agent_iscsi_deploy_mock.assert_called_once_with(task, mock.ANY) configure_local_boot_mock.assert_called_once_with( - task, 'some-root-uuid') + task, root_uuid='some-root-uuid', + efi_system_part_uuid=None) + reboot_and_finish_deploy_mock.assert_called_once_with(task) + + @mock.patch.object(agent_base_vendor.BaseAgentVendor, + 'reboot_and_finish_deploy') + @mock.patch.object(agent_base_vendor.BaseAgentVendor, + 'configure_local_boot') + @mock.patch.object(iscsi_deploy, 'do_agent_iscsi_deploy') + @mock.patch.object(ilo_common, 'cleanup_vmedia_boot') + def test_continue_deploy_localboot_uefi(self, cleanup_vmedia_boot_mock, + do_agent_iscsi_deploy_mock, + configure_local_boot_mock, + reboot_and_finish_deploy_mock): + self.node.provision_state = states.DEPLOYWAIT + self.node.target_provision_state = states.DEPLOYING + self.node.instance_info = { + 'capabilities': {'boot_option': 'local'}} + self.node.save() + do_agent_iscsi_deploy_mock.return_value = { + 'root uuid': 'some-root-uuid', + 'efi system partition uuid': 'efi-system-part-uuid'} + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.vendor.continue_deploy(task) + cleanup_vmedia_boot_mock.assert_called_once_with(task) + do_agent_iscsi_deploy_mock.assert_called_once_with(task, + mock.ANY) + configure_local_boot_mock.assert_called_once_with( + task, root_uuid='some-root-uuid', + efi_system_part_uuid='efi-system-part-uuid') reboot_and_finish_deploy_mock.assert_called_once_with(task) diff --git a/ironic/tests/drivers/test_agent_base_vendor.py b/ironic/tests/drivers/test_agent_base_vendor.py index e764b98713..ba70a9f3fc 100644 --- a/ironic/tests/drivers/test_agent_base_vendor.py +++ b/ironic/tests/drivers/test_agent_base_vendor.py @@ -328,19 +328,37 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) - @mock.patch.object(deploy_utils, 'try_set_boot_device') @mock.patch.object(agent_client.AgentClient, 'install_bootloader') - def test_configure_local_boot(self, install_bootloader_mock, - try_set_boot_device_mock): + @mock.patch.object(deploy_utils, 'try_set_boot_device') + def test_configure_local_boot(self, try_set_boot_device_mock, + install_bootloader_mock): install_bootloader_mock.return_value = { 'command_status': 'SUCCESS', 'command_error': None} with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: self.passthru.configure_local_boot(task, 'some-root-uuid') - install_bootloader_mock.assert_called_once_with( - task.node, 'some-root-uuid') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK) + install_bootloader_mock.assert_called_once_with( + task.node, root_uuid='some-root-uuid', + efi_system_part_uuid=None) + + @mock.patch.object(agent_client.AgentClient, 'install_bootloader') + @mock.patch.object(deploy_utils, 'try_set_boot_device') + def test_configure_local_boot_uefi(self, try_set_boot_device_mock, + install_bootloader_mock): + install_bootloader_mock.return_value = { + 'command_status': 'SUCCESS', 'command_error': None} + with task_manager.acquire(self.context, self.node['uuid'], + shared=False) as task: + self.passthru.configure_local_boot( + task, root_uuid='some-root-uuid', + efi_system_part_uuid='efi-system-part-uuid') + try_set_boot_device_mock.assert_called_once_with( + task, boot_devices.DISK) + install_bootloader_mock.assert_called_once_with( + task.node, root_uuid='some-root-uuid', + efi_system_part_uuid='efi-system-part-uuid') @mock.patch.object(agent_client.AgentClient, 'install_bootloader') def test_configure_local_boot_boot_loader_install_fail( @@ -356,7 +374,8 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.passthru.configure_local_boot, task, 'some-root-uuid') install_bootloader_mock.assert_called_once_with( - task.node, 'some-root-uuid') + task.node, root_uuid='some-root-uuid', + efi_system_part_uuid=None) self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) @@ -376,7 +395,8 @@ class TestBaseAgentVendor(db_base.DbTestCase): self.passthru.configure_local_boot, task, 'some-root-uuid') install_bootloader_mock.assert_called_once_with( - task.node, 'some-root-uuid') + task.node, root_uuid='some-root-uuid', + efi_system_part_uuid=None) try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK) self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) diff --git a/ironic/tests/drivers/test_agent_client.py b/ironic/tests/drivers/test_agent_client.py index 7c339ee7f5..b8e1fbf85a 100644 --- a/ironic/tests/drivers/test_agent_client.py +++ b/ironic/tests/drivers/test_agent_client.py @@ -136,13 +136,15 @@ class TestAgentClient(base.TestCase): wait=True) @mock.patch('uuid.uuid4', mock.MagicMock(return_value='uuid')) - def test_start_install_bootloader(self): + def test_install_bootloader(self): self.client._command = mock.Mock() root_uuid = 'fake-root-uuid' - params = {'root_uuid': root_uuid} + efi_system_part_uuid = 'fake-efi-system-part-uuid' + params = {'root_uuid': root_uuid, + 'efi_system_part_uuid': efi_system_part_uuid} - self.client.install_bootloader(self.node, root_uuid) - self.client._command.assert_called_once_with(node=self.node, - method='image.install_bootloader', - params=params, - wait=True) + self.client.install_bootloader( + self.node, root_uuid, efi_system_part_uuid=efi_system_part_uuid) + self.client._command.assert_called_once_with( + node=self.node, method='image.install_bootloader', params=params, + wait=True) diff --git a/ironic/tests/drivers/test_deploy_utils.py b/ironic/tests/drivers/test_deploy_utils.py index 85b197e3ca..29731c176f 100644 --- a/ironic/tests/drivers/test_deploy_utils.py +++ b/ironic/tests/drivers/test_deploy_utils.py @@ -300,12 +300,15 @@ class PhysicalWorkTestCase(tests_base.TestCase): mock.call.logout_iscsi(address, port, iqn), mock.call.delete_iscsi(address, port, iqn)] - returned_root_uuid = utils.deploy_partition_image( + uuids_dict_returned = utils.deploy_partition_image( address, port, iqn, lun, image_path, root_mb, swap_mb, ephemeral_mb, ephemeral_format, node_uuid, **deploy_kwargs) self.assertEqual(calls_expected, parent_mock.mock_calls) - self.assertEqual(root_uuid, returned_root_uuid) + expected_uuid_dict = { + 'root uuid': root_uuid, + 'efi system partition uuid': None} + self.assertEqual(expected_uuid_dict, uuids_dict_returned) def test_deploy_partition_image_without_boot_option(self): self._test_deploy_partition_image() @@ -331,7 +334,10 @@ class PhysicalWorkTestCase(tests_base.TestCase): self._test_deploy_partition_image(boot_option="netboot", boot_mode="uefi") - def test_deploy_partition_image_localboot_uefi(self): + # We mock utils.block_uuid separately here because we can't predict + # the order in which it will be called. + @mock.patch.object(utils, 'block_uuid') + def test_deploy_partition_image_localboot_uefi(self, block_uuid_mock): """Check loosely all functions are called with right args.""" address = '127.0.0.1' port = 3306 @@ -350,16 +356,24 @@ class PhysicalWorkTestCase(tests_base.TestCase): root_part = '/dev/fake-part3' efi_system_part = '/dev/fake-part1' root_uuid = '12345678-1234-1234-12345678-12345678abcdef' + efi_system_part_uuid = '9036-482' name_list = ['get_dev', 'get_image_mb', 'discovery', 'login_iscsi', 'logout_iscsi', 'delete_iscsi', 'make_partitions', 'is_block_device', 'populate_image', 'mkfs', - 'block_uuid', 'notify', 'destroy_disk_metadata'] + 'notify', 'destroy_disk_metadata'] parent_mock = self._mock_calls(name_list) parent_mock.get_dev.return_value = dev parent_mock.get_image_mb.return_value = 1 parent_mock.is_block_device.return_value = True - parent_mock.block_uuid.return_value = root_uuid + + def block_uuid_side_effect(device): + if device == root_part: + return root_uuid + if device == efi_system_part: + return efi_system_part_uuid + + block_uuid_mock.side_effect = block_uuid_side_effect parent_mock.make_partitions.return_value = { 'root': root_part, 'swap': swap_part, 'efi system partition': efi_system_part} @@ -385,17 +399,21 @@ class PhysicalWorkTestCase(tests_base.TestCase): mock.call.populate_image(image_path, root_part), mock.call.mkfs(dev=swap_part, fs='swap', label='swap1'), - mock.call.block_uuid(root_part), mock.call.logout_iscsi(address, port, iqn), mock.call.delete_iscsi(address, port, iqn)] - returned_root_uuid = utils.deploy_partition_image( + uuid_dict_returned = utils.deploy_partition_image( address, port, iqn, lun, image_path, root_mb, swap_mb, ephemeral_mb, ephemeral_format, node_uuid, boot_option="local", boot_mode="uefi") self.assertEqual(calls_expected, parent_mock.mock_calls) - self.assertEqual(root_uuid, returned_root_uuid) + block_uuid_mock.assert_any_call('/dev/fake-part1') + block_uuid_mock.assert_any_call('/dev/fake-part3') + expected_uuid_dict = { + 'root uuid': root_uuid, + 'efi system partition uuid': efi_system_part_uuid} + self.assertEqual(expected_uuid_dict, uuid_dict_returned) def test_deploy_partition_image_without_swap(self): """Check loosely all functions are called with right args.""" @@ -443,7 +461,7 @@ class PhysicalWorkTestCase(tests_base.TestCase): mock.call.logout_iscsi(address, port, iqn), mock.call.delete_iscsi(address, port, iqn)] - returned_root_uuid = utils.deploy_partition_image(address, port, iqn, + uuid_dict_returned = utils.deploy_partition_image(address, port, iqn, lun, image_path, root_mb, swap_mb, ephemeral_mb, @@ -451,7 +469,7 @@ class PhysicalWorkTestCase(tests_base.TestCase): node_uuid) self.assertEqual(calls_expected, parent_mock.mock_calls) - self.assertEqual(root_uuid, returned_root_uuid) + self.assertEqual(root_uuid, uuid_dict_returned['root uuid']) def test_deploy_partition_image_with_ephemeral(self): """Check loosely all functions are called with right args.""" @@ -510,7 +528,7 @@ class PhysicalWorkTestCase(tests_base.TestCase): mock.call.logout_iscsi(address, port, iqn), mock.call.delete_iscsi(address, port, iqn)] - returned_root_uuid = utils.deploy_partition_image(address, port, iqn, + uuid_dict_returned = utils.deploy_partition_image(address, port, iqn, lun, image_path, root_mb, swap_mb, ephemeral_mb, @@ -518,7 +536,7 @@ class PhysicalWorkTestCase(tests_base.TestCase): node_uuid) self.assertEqual(calls_expected, parent_mock.mock_calls) - self.assertEqual(root_uuid, returned_root_uuid) + self.assertEqual(root_uuid, uuid_dict_returned['root uuid']) def test_deploy_partition_image_preserve_ephemeral(self): """Check if all functions are called with right args.""" @@ -574,17 +592,13 @@ class PhysicalWorkTestCase(tests_base.TestCase): mock.call.logout_iscsi(address, port, iqn), mock.call.delete_iscsi(address, port, iqn)] - returned_root_uuid = utils.deploy_partition_image(address, port, iqn, - lun, image_path, - root_mb, swap_mb, - ephemeral_mb, - ephemeral_format, - node_uuid, - preserve_ephemeral=True, - boot_option="netboot") + uuid_dict_returned = utils.deploy_partition_image( + address, port, iqn, lun, image_path, root_mb, swap_mb, + ephemeral_mb, ephemeral_format, node_uuid, + preserve_ephemeral=True, boot_option="netboot") self.assertEqual(calls_expected, parent_mock.mock_calls) self.assertFalse(parent_mock.get_dev_block_size.called) - self.assertEqual(root_uuid, returned_root_uuid) + self.assertEqual(root_uuid, uuid_dict_returned['root uuid']) @mock.patch.object(common_utils, 'unlink_without_raise') def test_deploy_partition_image_with_configdrive(self, mock_unlink): @@ -643,16 +657,13 @@ class PhysicalWorkTestCase(tests_base.TestCase): mock.call.logout_iscsi(address, port, iqn), mock.call.delete_iscsi(address, port, iqn)] - returned_root_uuid = utils.deploy_partition_image(address, port, iqn, - lun, image_path, - root_mb, swap_mb, - ephemeral_mb, - ephemeral_format, - node_uuid, - configdrive=configdrive_url) + uuid_dict_returned = utils.deploy_partition_image( + address, port, iqn, lun, image_path, root_mb, swap_mb, + ephemeral_mb, ephemeral_format, node_uuid, + configdrive=configdrive_url) self.assertEqual(calls_expected, parent_mock.mock_calls) - self.assertEqual(root_uuid, returned_root_uuid) + self.assertEqual(root_uuid, uuid_dict_returned['root uuid']) mock_unlink.assert_called_once_with('configdrive-path') @mock.patch.object(utils, 'get_disk_identifier') @@ -681,10 +692,11 @@ class PhysicalWorkTestCase(tests_base.TestCase): mock.call.logout_iscsi(address, port, iqn), mock.call.delete_iscsi(address, port, iqn)] - utils.deploy_disk_image(address, port, iqn, lun, - image_path, node_uuid) + uuid_dict_returned = utils.deploy_disk_image(address, port, iqn, lun, + image_path, node_uuid) self.assertEqual(calls_expected, parent_mock.mock_calls) + self.assertEqual('0x12345678', uuid_dict_returned['disk identifier']) @mock.patch.object(common_utils, 'execute') def test_verify_iscsi_connection_raises(self, mock_exec): diff --git a/ironic/tests/drivers/test_iscsi_deploy.py b/ironic/tests/drivers/test_iscsi_deploy.py index 46329862f2..e616653f7b 100644 --- a/ironic/tests/drivers/test_iscsi_deploy.py +++ b/ironic/tests/drivers/test_iscsi_deploy.py @@ -533,7 +533,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - iscsi_deploy.continue_deploy(task, **kwargs) + retval = iscsi_deploy.continue_deploy(task, **kwargs) self.assertIsNotNone(task.node.last_error) self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) @@ -541,6 +541,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): mock_image_cache.assert_called_once_with() mock_image_cache.return_value.clean_up.assert_called_once_with() self.assertFalse(deploy_mock.called) + self.assertEqual({}, retval) @mock.patch.object(iscsi_deploy, 'LOG') @mock.patch.object(iscsi_deploy, 'get_deploy_info') @@ -577,10 +578,13 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): 'node': self.node.uuid, 'params': log_params, } + uuid_dict_returned = {'root uuid': '12345678-87654321'} + deploy_mock.return_value = uuid_dict_returned + with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: mock_log.isEnabledFor.return_value = True - iscsi_deploy.continue_deploy(task, **kwargs) + retval = iscsi_deploy.continue_deploy(task, **kwargs) mock_log.debug.assert_called_once_with( mock.ANY, expected_dict) self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) @@ -588,6 +592,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): self.assertIsNone(task.node.last_error) mock_image_cache.assert_called_once_with() mock_image_cache.return_value.clean_up.assert_called_once_with() + self.assertEqual(uuid_dict_returned, retval) @mock.patch.object(iscsi_deploy, 'LOG') @mock.patch.object(iscsi_deploy, 'get_deploy_info') @@ -617,11 +622,13 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): 'node': self.node.uuid, 'params': log_params, } + uuid_dict_returned = {'disk identifier': '87654321'} + deploy_mock.return_value = uuid_dict_returned with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = True mock_log.isEnabledFor.return_value = True - iscsi_deploy.continue_deploy(task, **kwargs) + retval = iscsi_deploy.continue_deploy(task, **kwargs) mock_log.debug.assert_called_once_with( mock.ANY, expected_dict) self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) @@ -629,6 +636,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): self.assertIsNone(task.node.last_error) mock_image_cache.assert_called_once_with() mock_image_cache.return_value.clean_up.assert_called_once_with() + self.assertEqual(uuid_dict_returned, retval) def test_get_deploy_info_boot_option_default(self): instance_info = self.node.instance_info @@ -674,7 +682,8 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): driver_internal_info = {'agent_url': 'http://1.2.3.4:1234'} self.node.driver_internal_info = driver_internal_info self.node.save() - continue_deploy_mock.return_value = 'some-root-uuid' + uuid_dict_returned = {'root uuid': 'some-root-uuid'} + continue_deploy_mock.return_value = uuid_dict_returned with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: @@ -686,10 +695,10 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): continue_deploy_mock.assert_called_once_with( task, error=None, iqn='iqn-qweqwe', key='abcdef', address='1.2.3.4') - self.assertEqual('some-root-uuid', ret_val) self.assertEqual( - 'some-root-uuid', - task.node.driver_internal_info['root_uuid_or_disk_id']) + 'some-root-uuid', + task.node.driver_internal_info['root_uuid_or_disk_id']) + self.assertEqual(ret_val, uuid_dict_returned) @mock.patch.object(iscsi_deploy, 'build_deploy_ramdisk_options') def test_do_agent_iscsi_deploy_start_iscsi_failure(self, @@ -730,7 +739,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() - continue_deploy_mock.return_value = None + continue_deploy_mock.return_value = {} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: diff --git a/ironic/tests/drivers/test_pxe.py b/ironic/tests/drivers/test_pxe.py index 56586c37ac..03c699668b 100644 --- a/ironic/tests/drivers/test_pxe.py +++ b/ironic/tests/drivers/test_pxe.py @@ -826,7 +826,7 @@ class PXEDriverTestCase(db_base.DbTestCase): self.node.save() root_uuid = "12345678-1234-1234-1234-1234567890abcxyz" - mock_deploy.return_value = root_uuid + mock_deploy.return_value = {'root uuid': root_uuid} boot_mode = None is_whole_disk_image = False @@ -887,7 +887,7 @@ class PXEDriverTestCase(db_base.DbTestCase): boot_mode = None is_whole_disk_image = True disk_id = '0x12345678' - mock_deploy.return_value = disk_id + mock_deploy.return_value = {'disk identifier': disk_id} with task_manager.acquire(self.context, self.node.uuid) as task: task.node.driver_internal_info['is_whole_disk_image'] = True @@ -1137,7 +1137,8 @@ class TestAgentVendorPassthru(db_base.DbTestCase): switch_pxe_config_mock, reboot_and_finish_deploy_mock): - do_agent_iscsi_deploy_mock.return_value = 'some-root-uuid' + uuid_dict_returned = {'root uuid': 'some-root-uuid'} + do_agent_iscsi_deploy_mock.return_value = uuid_dict_returned self.driver.vendor.continue_deploy(self.task) destroy_token_file_mock.assert_called_once_with(self.node) do_agent_iscsi_deploy_mock.assert_called_once_with( @@ -1164,13 +1165,44 @@ class TestAgentVendorPassthru(db_base.DbTestCase): self.node.instance_info = { 'capabilities': {'boot_option': 'local'}} self.node.save() - do_agent_iscsi_deploy_mock.return_value = 'some-root-uuid' + uuid_dict_returned = {'root uuid': 'some-root-uuid'} + do_agent_iscsi_deploy_mock.return_value = uuid_dict_returned self.driver.vendor.continue_deploy(self.task) destroy_token_file_mock.assert_called_once_with(self.node) do_agent_iscsi_deploy_mock.assert_called_once_with( self.task, self.driver.vendor._client) - configure_local_boot_mock.assert_called_once_with(self.task, - 'some-root-uuid') + configure_local_boot_mock.assert_called_once_with( + self.task, root_uuid='some-root-uuid', efi_system_part_uuid=None) + clean_up_pxe_config_mock.assert_called_once_with(self.task) + reboot_and_finish_deploy_mock.assert_called_once_with(self.task) + + @mock.patch.object(agent_base_vendor.BaseAgentVendor, + 'reboot_and_finish_deploy') + @mock.patch.object(pxe_utils, 'clean_up_pxe_config') + @mock.patch.object(agent_base_vendor.BaseAgentVendor, + 'configure_local_boot') + @mock.patch.object(iscsi_deploy, 'do_agent_iscsi_deploy') + @mock.patch.object(pxe, '_destroy_token_file') + def test_continue_deploy_localboot_uefi(self, destroy_token_file_mock, + do_agent_iscsi_deploy_mock, + configure_local_boot_mock, + clean_up_pxe_config_mock, + reboot_and_finish_deploy_mock): + + self.node.instance_info = { + 'capabilities': {'boot_option': 'local'}} + self.node.save() + uuid_dict_returned = {'root uuid': 'some-root-uuid', + 'efi system partition uuid': 'efi-part-uuid'} + do_agent_iscsi_deploy_mock.return_value = uuid_dict_returned + + self.driver.vendor.continue_deploy(self.task) + destroy_token_file_mock.assert_called_once_with(self.node) + do_agent_iscsi_deploy_mock.assert_called_once_with( + self.task, self.driver.vendor._client) + configure_local_boot_mock.assert_called_once_with( + self.task, root_uuid='some-root-uuid', + efi_system_part_uuid='efi-part-uuid') clean_up_pxe_config_mock.assert_called_once_with(self.task) reboot_and_finish_deploy_mock.assert_called_once_with(self.task)