diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py index dd5ffda1c..36be96d3e 100644 --- a/ironic_python_agent/extensions/standby.py +++ b/ironic_python_agent/extensions/standby.py @@ -17,6 +17,7 @@ import os import time from ironic_lib import disk_utils +from ironic_lib import exception from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log @@ -359,6 +360,40 @@ def _validate_image_info(ext, image_info=None, **kwargs): 'Image \'os_hash_value\' must be a non-empty string.') +def _validate_partitioning(device): + """Validate the final partition table. + + Check if after writing the image to disk we have a valid partition + table by trying to read it. This will fail if the disk is junk. + """ + try: + # Ensure we re-read the partition table before we try to list + # partitions + utils.execute('partprobe', device, run_as_root=True, + attempts=CONF.disk_utils.partprobe_attempts) + except (processutils.UnknownArgumentError, + processutils.ProcessExecutionError, OSError) as e: + LOG.warning("Unable to probe for partitions on device %(device)s " + "after writing the image, the partitioning table may " + "be broken. Error: %(error)s", + {'device': device, 'error': e}) + + try: + nparts = len(disk_utils.list_partitions(device)) + except (processutils.UnknownArgumentError, + processutils.ProcessExecutionError, OSError) as e: + msg = ("Unable to find a valid partition table on the disk after " + "writing the image. Error {}".format(e)) + raise exception.InstanceDeployFailure(msg) + + # Check if there is at least one partition in the partition table after + # deploy + if not nparts: + msg = ("No partitions found on the device {} after writing " + "the image.".format(device)) + raise exception.InstanceDeployFailure(msg) + + class StandbyExtension(base.BaseAgentExtension): """Extension which adds stand-by related functionality to agent.""" def __init__(self, agent=None): @@ -495,6 +530,8 @@ class StandbyExtension(base.BaseAgentExtension): else: self._cache_and_write_image(image_info, device) + _validate_partitioning(device) + # the configdrive creation is taken care by ironic-lib's # work_on_disk(). if image_info.get('image_type') != 'partition': diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py index a69aec580..2b5e7b66d 100644 --- a/ironic_python_agent/tests/unit/extensions/test_standby.py +++ b/ironic_python_agent/tests/unit/extensions/test_standby.py @@ -603,6 +603,10 @@ class TestStandbyExtension(base.IronicAgentTest): @mock.patch('ironic_lib.disk_utils.get_disk_identifier', lambda dev: 'ROOT') + @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', @@ -615,12 +619,15 @@ class TestStandbyExtension(base.IronicAgentTest): download_mock, write_mock, dispatch_mock, - configdrive_copy_mock): + configdrive_copy_mock, + list_part_mock, + execute_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, @@ -640,7 +647,14 @@ class TestStandbyExtension(base.IronicAgentTest): cmd_result = ('prepare_image: image ({}) written to device {} ' 'root_uuid=ROOT').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) + @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', @@ -653,12 +667,15 @@ class TestStandbyExtension(base.IronicAgentTest): download_mock, write_mock, dispatch_mock, - configdrive_copy_mock): + configdrive_copy_mock, + list_part_mock, + execute_mock): image_info = _build_fake_partition_image_info() download_mock.return_value = None write_mock.return_value = {'root uuid': 'root_uuid'} 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, @@ -698,11 +715,18 @@ class TestStandbyExtension(base.IronicAgentTest): 'root_uuid={}').format( image_info['id'], 'manager', 'root_uuid') 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) @mock.patch('ironic_lib.disk_utils.get_disk_identifier', lambda dev: 'ROOT') + @mock.patch('ironic_python_agent.utils.execute', autospec=True) @mock.patch('ironic_lib.disk_utils.create_config_drive_partition', autospec=True) + @mock.patch('ironic_lib.disk_utils.list_partitions', + autospec=True) @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', autospec=True) @mock.patch('ironic_python_agent.extensions.standby._write_image', @@ -713,12 +737,15 @@ class TestStandbyExtension(base.IronicAgentTest): download_mock, write_mock, dispatch_mock, - configdrive_copy_mock): + list_part_mock, + configdrive_copy_mock, + execute_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, @@ -740,6 +767,55 @@ class TestStandbyExtension(base.IronicAgentTest): @mock.patch('ironic_lib.disk_utils.get_disk_identifier', lambda dev: 'ROOT') @mock.patch('ironic_lib.disk_utils.work_on_disk', autospec=True) + @mock.patch('ironic_lib.disk_utils.create_config_drive_partition', + autospec=True) + @mock.patch('ironic_lib.disk_utils.list_partitions', + 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_bad_partition(self, + download_mock, + write_mock, + dispatch_mock, + list_part_mock, + configdrive_copy_mock, + work_on_disk_mock): + list_part_mock.side_effect = processutils.ProcessExecutionError + 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 + work_on_disk_mock.return_value = { + 'root uuid': 'a318821b-2a60-40e5-a011-7ac07fce342b', + 'partitions': { + 'root': '/dev/foo-part1', + } + } + + async_result = self.agent_extension.prepare_image( + image_info=image_info, + configdrive=None + ) + 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') + + self.assertFalse(configdrive_copy_mock.called) + self.assertEqual('FAILED', async_result.command_status) + + @mock.patch('ironic_python_agent.utils.execute', mock.Mock()) + @mock.patch('ironic_lib.disk_utils.list_partitions', + lambda _dev: [mock.Mock()]) + @mock.patch('ironic_lib.disk_utils.get_disk_identifier', + lambda dev: 'ROOT') + @mock.patch('ironic_lib.disk_utils.work_on_disk', autospec=True) @mock.patch('ironic_lib.disk_utils.create_config_drive_partition', autospec=True) @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', diff --git a/releasenotes/notes/check-partition-table-after-writing-34efbd557d8de7cb.yaml b/releasenotes/notes/check-partition-table-after-writing-34efbd557d8de7cb.yaml new file mode 100644 index 000000000..e0157a3c5 --- /dev/null +++ b/releasenotes/notes/check-partition-table-after-writing-34efbd557d8de7cb.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes a bug with bad whole disk images and config drives, where we would + attempt to write a config drive partition to the disk without a valid + partition table.