From 6d7ec350ff3f880fa85cb58da91216053e4b4375 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 17 Jun 2020 14:38:58 +0200 Subject: [PATCH] Make get_partition_uuids work with whole disk images We used to popular root UUID inside the message formatting function, move it to actual prepare_image/cache_image calls. Change-Id: Ifb22220dfd49633e8623dd76f7a6a128f5874b78 --- ironic_python_agent/extensions/standby.py | 57 +++++++------ .../tests/unit/extensions/test_standby.py | 85 +++++++++++++------ 2 files changed, 90 insertions(+), 52 deletions(-) diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py index d66216341..d238d76f5 100644 --- a/ironic_python_agent/extensions/standby.py +++ b/ironic_python_agent/extensions/standby.py @@ -224,31 +224,21 @@ def _message_format(msg, image_info, device, partition_uuids): """Helper method to get and populate different messages.""" message = None result_msg = msg - if image_info.get('image_type') == 'partition': - root_uuid = partition_uuids.get('root uuid') - efi_system_partition_uuid = ( - partition_uuids.get('efi system partition uuid')) - if (image_info.get('deploy_boot_mode') == 'uefi' - and image_info.get('boot_option') == 'local'): - result_msg = msg + 'root_uuid={} efi_system_partition_uuid={}' - message = result_msg.format(image_info['id'], device, - root_uuid, - efi_system_partition_uuid) - else: - result_msg = msg + 'root_uuid={}' - message = result_msg.format(image_info['id'], device, root_uuid) + + root_uuid = partition_uuids.get('root uuid') + efi_system_partition_uuid = ( + partition_uuids.get('efi system partition uuid')) + if (image_info.get('deploy_boot_mode') == 'uefi' + and image_info.get('boot_option') == 'local' + and efi_system_partition_uuid): + result_msg = msg + 'root_uuid={} efi_system_partition_uuid={}' + message = result_msg.format(image_info['id'], device, + root_uuid, + efi_system_partition_uuid) else: - try: - # NOTE(TheJulia): ironic-lib disk_utils.get_disk_identifier - # can raise OSError if hexdump is not found. - root_uuid = disk_utils.get_disk_identifier(device) - result_msg = msg + 'root_uuid={}' - message = result_msg.format(image_info['id'], device, root_uuid) - except OSError as e: - LOG.warning('Failed to call get_disk_identifier: ' - 'Unable to obtain the root_uuid parameter: ' - 'The hexdump tool may be missing in IPA: %s', e) - message = result_msg.format(image_info['id'], device) + result_msg = msg + 'root_uuid={}' + message = result_msg.format(image_info['id'], device, root_uuid) + return message @@ -546,6 +536,22 @@ class StandbyExtension(base.BaseAgentExtension): # Note: the catch internal to the helper method logs any errors. pass + def _fix_up_partition_uuids(self, image_info, device): + if self.partition_uuids is None: + self.partition_uuids = {} + + if image_info.get('image_type') == 'partition': + return + + try: + root_uuid = disk_utils.get_disk_identifier(device) + except OSError as e: + LOG.warning('Failed to call get_disk_identifier: ' + 'Unable to obtain the root_uuid parameter: ' + 'The hexdump tool may be missing in IPA: %s', e) + else: + self.partition_uuids['root uuid'] = root_uuid + @base.async_command('cache_image', _validate_image_info) def cache_image(self, image_info=None, force=False): """Asynchronously caches specified image to the local OS device. @@ -571,6 +577,7 @@ class StandbyExtension(base.BaseAgentExtension): self._cache_and_write_image(image_info, device) msg = 'image ({}) cached to device {} ' + self._fix_up_partition_uuids(image_info, device) result_msg = _message_format(msg, image_info, device, self.partition_uuids) @@ -640,6 +647,8 @@ class StandbyExtension(base.BaseAgentExtension): disk_utils.create_config_drive_partition(node_uuid, device, configdrive) + + self._fix_up_partition_uuids(image_info, device) msg = 'image ({}) written to device {} ' result_msg = _message_format(msg, image_info, device, self.partition_uuids) diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py index f3d5bccdd..ff8738d1f 100644 --- a/ironic_python_agent/tests/unit/extensions/test_standby.py +++ b/ironic_python_agent/tests/unit/extensions/test_standby.py @@ -711,6 +711,8 @@ class TestStandbyExtension(base.IronicAgentTest): execute_mock.assert_called_with('partprobe', 'manager', run_as_root=True, attempts=mock.ANY) + self.assertEqual({'root uuid': 'ROOT'}, + self.agent_extension.partition_uuids) @mock.patch('ironic_python_agent.utils.execute', autospec=True) @mock.patch('ironic_lib.disk_utils.list_partitions', @@ -779,6 +781,8 @@ class TestStandbyExtension(base.IronicAgentTest): execute_mock.assert_called_with('partprobe', 'manager', run_as_root=True, attempts=mock.ANY) + self.assertEqual({'root uuid': 'root_uuid'}, + self.agent_extension.partition_uuids) @mock.patch('ironic_lib.disk_utils.get_disk_identifier', lambda dev: 'ROOT') @@ -870,6 +874,59 @@ class TestStandbyExtension(base.IronicAgentTest): self.assertFalse(configdrive_copy_mock.called) self.assertEqual('FAILED', async_result.command_status) + @mock.patch('ironic_lib.disk_utils.get_disk_identifier', + side_effect=OSError, autospec=True) + @mock.patch('ironic_python_agent.utils.execute', + autospec=True) + @mock.patch('ironic_lib.disk_utils.list_partitions', + autospec=True) + @mock.patch('ironic_lib.disk_utils.create_config_drive_partition', + autospec=True) + @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', + autospec=True) + @mock.patch('ironic_python_agent.extensions.standby._write_image', + autospec=True) + @mock.patch('ironic_python_agent.extensions.standby._download_image', + autospec=True) + def test_prepare_image_no_hexdump(self, + download_mock, + write_mock, + dispatch_mock, + configdrive_copy_mock, + list_part_mock, + execute_mock, + disk_id_mock): + image_info = _build_fake_image_info() + download_mock.return_value = None + write_mock.return_value = None + dispatch_mock.return_value = 'manager' + configdrive_copy_mock.return_value = None + list_part_mock.return_value = [mock.MagicMock()] + + async_result = self.agent_extension.prepare_image( + image_info=image_info, + configdrive='configdrive_data' + ) + async_result.join() + + download_mock.assert_called_once_with(image_info) + write_mock.assert_called_once_with(image_info, 'manager') + dispatch_mock.assert_called_once_with('get_os_install_device') + configdrive_copy_mock.assert_called_once_with(image_info['node_uuid'], + 'manager', + 'configdrive_data') + + self.assertEqual('SUCCEEDED', async_result.command_status) + self.assertIn('result', async_result.command_result) + cmd_result = ('prepare_image: image ({}) written to device {} ' + 'root_uuid=None').format(image_info['id'], 'manager') + self.assertEqual(cmd_result, async_result.command_result['result']) + list_part_mock.assert_called_with('manager') + execute_mock.assert_called_with('partprobe', 'manager', + run_as_root=True, + attempts=mock.ANY) + self.assertEqual({}, self.agent_extension.partition_uuids) + @mock.patch('ironic_python_agent.utils.execute', mock.Mock()) @mock.patch('ironic_lib.disk_utils.list_partitions', lambda _dev: [mock.Mock()]) @@ -1148,19 +1205,6 @@ class TestStandbyExtension(base.IronicAgentTest): # Assert write was only called once and failed! file_mock.write.assert_called_once_with('some') - @mock.patch('ironic_lib.disk_utils.get_disk_identifier', - lambda dev: 'ROOT') - def test__message_format_whole_disk(self): - image_info = _build_fake_image_info() - msg = 'image ({}) already present on device {} ' - device = '/dev/fake' - partition_uuids = {} - result_msg = standby._message_format(msg, image_info, - device, partition_uuids) - expected_msg = ('image (fake_id) already present on device ' - '/dev/fake root_uuid=ROOT') - self.assertEqual(expected_msg, result_msg) - def test__message_format_partition_bios(self): image_info = _build_fake_partition_image_info() msg = ('image ({}) already present on device {} ') @@ -1202,21 +1246,6 @@ class TestStandbyExtension(base.IronicAgentTest): 'efi_system_partition_uuid=efi_id') self.assertEqual(expected_msg, result_msg) - @mock.patch('ironic_lib.disk_utils.get_disk_identifier', - autospec=True) - def test__message_format_whole_disk_missing_oserror(self, - ident_mock): - ident_mock.side_effect = OSError - image_info = _build_fake_image_info() - msg = 'image ({}) already present on device {}' - device = '/dev/fake' - partition_uuids = {} - result_msg = standby._message_format(msg, image_info, - device, partition_uuids) - expected_msg = ('image (fake_id) already present on device ' - '/dev/fake') - self.assertEqual(expected_msg, result_msg) - @mock.patch('ironic_python_agent.utils.determine_time_method', autospec=True) @mock.patch('ironic_python_agent.utils.execute', autospec=True)