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
This commit is contained in:
Sam Betts 2018-03-29 13:17:45 +01:00 committed by Dmitry Tantsur
parent f63099ebb6
commit fc2dfcee60
3 changed files with 122 additions and 3 deletions

View File

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

View File

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

View File

@ -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.