Merge "Use a pre-defined partition UUID to detect configdrive on GPT" into stable/xena

Author: Zuul

ironic_python_agent/partition_utils.py

@@ -33,8 +33,11 @@
 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()
 

ironic_python_agent/tests/unit/test_partition_utils.py

@@ -22,6 +22,7 @@
 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 @@ def test_create_partition_exists(self, mock_get_configdrive,
         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 @@ def test_create_partition_exists(self, mock_get_configdrive,
                        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 @@ def test_create_partition_gpt(self, mock_get_configdrive,
                       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)

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.