From 1ec0f70747def2db15466397b41027f9bd0c64c4 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 29 Apr 2022 15:01:02 +0200 Subject: [PATCH] Use a pre-defined partition UUID to detect configdrive on GPT Using partition numbers is currently broken for devicemapper devices. Fortunately, GPT has partition UUIDs, so we can just generate one and use it for lookup. NOTE on backport: _get_partition is imported from the image extension. In Yoga it was moved to partition_utils. Change-Id: I41ffe4f8e4c6e43182090b5aa2a2b4b34f32efd5 (cherry picked from commit 65c4de903a2d8059b1520b0102235b6700287f87) --- ironic_python_agent/partition_utils.py | 47 +++++++++++++------ .../tests/unit/test_partition_utils.py | 33 ++++--------- ...configdrive-partuuid-3259cfb7428c1483.yaml | 17 +++++++ 3 files changed, 58 insertions(+), 39 deletions(-) create mode 100644 releasenotes/notes/configdrive-partuuid-3259cfb7428c1483.yaml diff --git a/ironic_python_agent/partition_utils.py b/ironic_python_agent/partition_utils.py index 8e03919bd..e2094229b 100644 --- a/ironic_python_agent/partition_utils.py +++ b/ironic_python_agent/partition_utils.py @@ -33,8 +33,11 @@ from oslo_config import cfg from oslo_log import log from oslo_utils import excutils from oslo_utils import units +from oslo_utils import uuidutils import requests +from ironic_python_agent import errors +from ironic_python_agent.extensions import image from ironic_python_agent import utils as ipa_utils @@ -373,14 +376,18 @@ def create_config_drive_partition(node_uuid, device, configdrive): "%(part)s", {'node': node_uuid, 'part': config_drive_part}) else: - cur_parts = set(part['number'] - for part in disk_utils.list_partitions(device)) - + part_uuid = None if disk_utils.get_partition_table_type(device) == 'gpt': + part_uuid = uuidutils.generate_uuid() create_option = '0:-%dMB:0' % MAX_CONFIG_DRIVE_SIZE_MB - utils.execute('sgdisk', '-n', create_option, device, + uuid_option = '0:%s' % part_uuid + utils.execute('sgdisk', '-n', create_option, + '-u', uuid_option, device, run_as_root=True) else: + cur_parts = set(part['number'] + for part in disk_utils.list_partitions(device)) + # Check if the disk has 4 partitions. The MBR based disk # cannot have more than 4 partitions. # TODO(stendulker): One can use logical partitions to create @@ -422,17 +429,29 @@ def create_config_drive_partition(node_uuid, device, configdrive): # Trigger device rescan disk_utils.trigger_device_rescan(device) - upd_parts = set(part['number'] - for part in disk_utils.list_partitions(device)) - new_part = set(upd_parts) - set(cur_parts) - if len(new_part) != 1: - raise exception.InstanceDeployFailure( - 'Disk partitioning failed on device %(device)s. ' - 'Unable to retrieve config drive partition information.' - % {'device': device}) + if part_uuid is None: + new_parts = {part['number']: part + for part in disk_utils.list_partitions(device)} + new_part = set(new_parts) - set(cur_parts) + if len(new_part) != 1: + raise exception.InstanceDeployFailure( + 'Disk partitioning failed on device %(device)s. ' + 'Unable to retrieve config drive partition ' + 'information.' % {'device': device}) - config_drive_part = disk_utils.partition_index_to_path( - device, new_part.pop()) + config_drive_part = disk_utils.partition_index_to_path( + device, new_part.pop()) + else: + try: + config_drive_part = image._get_partition(device, part_uuid) + except errors.DeviceNotFound: + msg = ('Failed to create config drive on disk %(disk)s ' + 'for node %(node)s. Partition with UUID %(uuid)s ' + 'has not been found after creation.') % { + 'disk': device, 'node': node_uuid, + 'uuid': part_uuid} + LOG.error(msg) + raise exception.InstanceDeployFailure(msg) disk_utils.udev_settle() diff --git a/ironic_python_agent/tests/unit/test_partition_utils.py b/ironic_python_agent/tests/unit/test_partition_utils.py index 0064acde7..93e61c47c 100644 --- a/ironic_python_agent/tests/unit/test_partition_utils.py +++ b/ironic_python_agent/tests/unit/test_partition_utils.py @@ -22,6 +22,7 @@ from ironic_lib import utils from oslo_concurrency import processutils import requests +from ironic_python_agent.extensions import image from ironic_python_agent import partition_utils from ironic_python_agent.tests.unit import base @@ -654,6 +655,7 @@ class CreateConfigDriveTestCases(base.IronicAgentTest): mock_dd.assert_called_with(configdrive_file, configdrive_part) mock_unlink.assert_called_with(configdrive_file) + @mock.patch('oslo_utils.uuidutils.generate_uuid', lambda: 'fake-uuid') @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'unlink_without_raise', autospec=True) @@ -663,50 +665,32 @@ class CreateConfigDriveTestCases(base.IronicAgentTest): autospec=True) @mock.patch.object(disk_utils, 'get_partition_table_type', autospec=True) - @mock.patch.object(disk_utils, 'list_partitions', - autospec=True) + @mock.patch.object(image, '_get_partition', autospec=True) @mock.patch.object(partition_utils, 'get_labelled_partition', autospec=True) @mock.patch.object(partition_utils, 'get_configdrive', autospec=True) def test_create_partition_gpt(self, mock_get_configdrive, mock_get_labelled_partition, - mock_list_partitions, mock_table_type, + mock_get_partition_by_uuid, + mock_table_type, mock_fix_gpt_partition, mock_dd, mock_unlink, mock_execute): config_url = 'http://1.2.3.4/cd' configdrive_file = '/tmp/xyz' configdrive_mb = 10 - initial_partitions = [{'end': 49152, 'number': 1, 'start': 1, - 'flags': 'boot', 'filesystem': 'ext4', - 'size': 49151}, - {'end': 51099, 'number': 3, 'start': 49153, - 'flags': '', 'filesystem': '', 'size': 2046}, - {'end': 51099, 'number': 5, 'start': 49153, - 'flags': '', 'filesystem': '', 'size': 2046}] - updated_partitions = [{'end': 49152, 'number': 1, 'start': 1, - 'flags': 'boot', 'filesystem': 'ext4', - 'size': 49151}, - {'end': 51099, 'number': 3, 'start': 49153, - 'flags': '', 'filesystem': '', 'size': 2046}, - {'end': 51099, 'number': 4, 'start': 49153, - 'flags': '', 'filesystem': '', 'size': 2046}, - {'end': 51099, 'number': 5, 'start': 49153, - 'flags': '', 'filesystem': '', 'size': 2046}] - mock_get_configdrive.return_value = (configdrive_mb, configdrive_file) mock_get_labelled_partition.return_value = None mock_table_type.return_value = 'gpt' - mock_list_partitions.side_effect = [initial_partitions, - updated_partitions] expected_part = '/dev/fake4' + mock_get_partition_by_uuid.return_value = expected_part partition_utils.create_config_drive_partition(self.node_uuid, self.dev, config_url) mock_execute.assert_has_calls([ - mock.call('sgdisk', '-n', '0:-64MB:0', self.dev, - run_as_root=True), + mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid', + self.dev, run_as_root=True), mock.call('sync'), mock.call('udevadm', 'settle'), mock.call('partprobe', self.dev, attempts=10, run_as_root=True), @@ -717,7 +701,6 @@ class CreateConfigDriveTestCases(base.IronicAgentTest): delay_on_retry=True) ]) - self.assertEqual(2, mock_list_partitions.call_count) mock_table_type.assert_called_with(self.dev) mock_fix_gpt_partition.assert_called_with(self.dev, self.node_uuid) mock_dd.assert_called_with(configdrive_file, expected_part) diff --git a/releasenotes/notes/configdrive-partuuid-3259cfb7428c1483.yaml b/releasenotes/notes/configdrive-partuuid-3259cfb7428c1483.yaml new file mode 100644 index 000000000..953442738 --- /dev/null +++ b/releasenotes/notes/configdrive-partuuid-3259cfb7428c1483.yaml @@ -0,0 +1,17 @@ +--- +issues: + - | + Creating a configdrive partition on a devicemapper device (e.g. a multipath + storage device) with MBR partitioning may fail with the following error:: + + Command execution failed: Failed to create config drive on disk /dev/dm-0 + for node 168af30d-0fad-4d67-af99-b28b3238e977. Error: Unexpected error + while running command. + + Use GPT partitioning instead. +fixes: + - | + Fixes creating a configdrive partition on a devicemapper device (e.g. + a multipath storage device) with GPT partitioning. The newly created + partition is now detected by a pre-generated UUID rather than by comparing + partition numbers.