Attempt to read the partition table after writing an image

This patch adds code that tries to read the partition table after we've
successfully written an image to make sure the image that we wrote has a
valid partition table so we can more easily guarantee that what we've
written is bootable and not just junk. Without a valid partition table
writing a config drive will fail for whole disk images.

Co-Authored-By: Dmitry Tantsur <dtantsur@redhat.com>
Change-Id: I5cfd8c433a4db3e0d2d5086250e629d16234b7a4
Story: 2001760
Task: 12159

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':

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',

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.