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)

3 files changed, 59 insertions, 39 deletions

diff --git a/ironic_python_agent/partition_utils.py b/ironic_python_agent/partition_utils.py
index c32a994d..c012b024 100644
--- a/ironic_python_agent/partition_utils.py
+++ b/ironic_python_agent/partition_utils.py
@@ -32,8 +32,12 @@ from oslo_concurrency import processutils
 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
+
 
 LOG = log.getLogger()
 
@@ -352,14 +356,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
@@ -401,17 +409,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 1eb2cbbf..c0f63668 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
 
@@ -592,6 +593,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)
@@ -601,50 +603,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),
@@ -655,7 +639,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 00000000..95344273
--- /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.