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.

Change-Id: I41ffe4f8e4c6e43182090b5aa2a2b4b34f32efd5

3 files changed, 55 insertions, 38 deletions

diff --git a/ironic_python_agent/partition_utils.py b/ironic_python_agent/partition_utils.py
index 42b90445..d82d0072 100644
--- a/ironic_python_agent/partition_utils.py
+++ b/ironic_python_agent/partition_utils.py
@@ -35,6 +35,7 @@ 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
@@ -377,14 +378,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
@@ -426,17 +431,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 = 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 b4017c2b..000d98c4 100644
--- a/ironic_python_agent/tests/unit/test_partition_utils.py
+++ b/ironic_python_agent/tests/unit/test_partition_utils.py
@@ -656,6 +656,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)
@@ -665,7 +666,7 @@ class CreateConfigDriveTestCases(base.IronicAgentTest):
                        autospec=True)
     @mock.patch.object(disk_utils, 'get_partition_table_type',
                        autospec=True)
-    @mock.patch.object(disk_utils, 'list_partitions',
+    @mock.patch.object(partition_utils, 'get_partition',
                        autospec=True)
     @mock.patch.object(partition_utils, 'get_labelled_partition',
                        autospec=True)
@@ -673,42 +674,25 @@ class CreateConfigDriveTestCases(base.IronicAgentTest):
                        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),
@@ -719,7 +703,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.