Add new 'disk_label' capability

This patch is adding support for a new capability called 'disk_label',
in the same way users can opt-in the boot mode for the target machine
they should also be able to choose the disk label that will be used when
formating the hard drive to accommodate their use cases; e.g users
operating machines in BIOS mode will be able to overcome the 2TB
limitation for partition size that is imposed by the MBR.

Closes-Bug: #1548788
Depends-On: I307315b1b32c9bf0babd7e42d4e9bc2030884980
Change-Id: I538029fea9326de9c62fdf965469dae8915551be

ironic/drivers/modules/deploy_utils.py

@@ -93,6 +93,7 @@ SUPPORTED_CAPABILITIES = {
     'boot_mode': ('bios', 'uefi'),
     'secure_boot': ('true', 'false'),
     'trusted_boot': ('true', 'false'),
+    'disk_label': ('msdos', 'gpt'),
 }
 
 
@@ -303,7 +304,7 @@ def deploy_partition_image(
         address, port, iqn, lun, image_path,
         root_mb, swap_mb, ephemeral_mb, ephemeral_format, node_uuid,
         preserve_ephemeral=False, configdrive=None,
-        boot_option="netboot", boot_mode="bios"):
+        boot_option="netboot", boot_mode="bios", disk_label=None):
     """All-in-one function to deploy a partition image to a node.
 
     :param address: The iSCSI IP address.
@@ -325,6 +326,9 @@ def deploy_partition_image(
                         or configdrive HTTP URL.
     :param boot_option: Can be "local" or "netboot". "netboot" by default.
     :param boot_mode: Can be "bios" or "uefi". "bios" by default.
+    :param disk_label: The disk label to be used when creating the
+        partition table. Valid values are: "msdos", "gpt" or None; If None
+        Ironic will figure it out according to the boot_mode parameter.
     :raises: InstanceDeployFailure if image virtual size is bigger than root
         partition size.
     :returns: a dictionary containing the following keys:
@@ -346,7 +350,7 @@ def deploy_partition_image(
             dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, image_path,
             node_uuid, preserve_ephemeral=preserve_ephemeral,
             configdrive=configdrive, boot_option=boot_option,
-            boot_mode=boot_mode)
+            boot_mode=boot_mode, disk_label=disk_label)
 
     return uuid_dict_returned
 
@@ -742,6 +746,18 @@ def is_trusted_boot_requested(node):
     return trusted_boot == 'true'
 
 
+def get_disk_label(node):
+    """Return the disk label requested for deploy, if any.
+
+    :param node: a single Node.
+    :raises: InvalidParameterValue if the capabilities string is not a
+             dictionary or is malformed.
+    :returns: the disk label or None if no disk label was specified.
+    """
+    capabilities = parse_instance_info_capabilities(node)
+    return capabilities.get('disk_label')
+
+
 def get_boot_mode_for_deploy(node):
     """Returns the boot mode that would be used for deploy.
 

ironic/drivers/modules/iscsi_deploy.py

@@ -319,6 +319,11 @@ def get_deploy_info(node, **kwargs):
                        'boot_option': deploy_utils.get_boot_option(node),
                        'boot_mode': _get_boot_mode(node)})
 
+        # Append disk label if specified
+        disk_label = deploy_utils.get_disk_label(node)
+        if disk_label is not None:
+            params['disk_label'] = disk_label
+
     missing = [key for key in params if params[key] is None]
     if missing:
         raise exception.MissingParameterValue(

ironic/tests/unit/drivers/modules/test_deploy_utils.py

@@ -337,7 +337,7 @@ class PhysicalWorkTestCase(tests_base.TestCase):
         return parent_mock
 
     def _test_deploy_partition_image(self, boot_option=None,
-                                     boot_mode=None):
+                                     boot_mode=None, disk_label=None):
         """Check loosely all functions are called with right args."""
         address = '127.0.0.1'
         port = 3306
@@ -375,7 +375,8 @@ class PhysicalWorkTestCase(tests_base.TestCase):
 
         make_partitions_expected_args = [dev, root_mb, swap_mb, ephemeral_mb,
                                          configdrive_mb, node_uuid]
-        make_partitions_expected_kwargs = {'commit': True, 'disk_label': None}
+        make_partitions_expected_kwargs = {'commit': True,
+                                           'disk_label': disk_label}
         deploy_kwargs = {}
 
         if boot_option:
@@ -390,6 +391,9 @@ class PhysicalWorkTestCase(tests_base.TestCase):
         else:
             make_partitions_expected_kwargs['boot_mode'] = 'bios'
 
+        if disk_label:
+            deploy_kwargs['disk_label'] = disk_label
+
         # If no boot_option, then it should default to netboot.
         utils_calls_expected = [mock.call.get_dev(address, port, iqn, lun),
                                 mock.call.discovery(address, port),
@@ -447,6 +451,9 @@ class PhysicalWorkTestCase(tests_base.TestCase):
         self._test_deploy_partition_image(boot_option="netboot",
                                           boot_mode="uefi")
 
+    def test_deploy_partition_image_disk_label(self):
+        self._test_deploy_partition_image(disk_label='gpt')
+
     @mock.patch.object(disk_utils, 'get_image_mb', return_value=129,
                        autospec=True)
     def test_deploy_partition_image_image_exceeds_root_partition(self,
@@ -1039,7 +1046,8 @@ class PhysicalWorkTestCase(tests_base.TestCase):
                                          node_uuid, configdrive=None,
                                          preserve_ephemeral=False,
                                          boot_option="netboot",
-                                         boot_mode="bios")]
+                                         boot_mode="bios",
+                                         disk_label=None)]
 
         self.assertRaises(TestException, utils.deploy_partition_image,
                           address, port, iqn, lun, image_path,
@@ -1461,6 +1469,12 @@ class ParseInstanceInfoCapabilitiesTestCase(tests_base.TestCase):
         self.assertEqual(('true', 'false'),
                          utils.SUPPORTED_CAPABILITIES['trusted_boot'])
 
+    def test_get_disk_label(self):
+        inst_info = {'capabilities': {'disk_label': 'gpt', 'foo': 'bar'}}
+        self.node.instance_info = inst_info
+        result = utils.get_disk_label(self.node)
+        self.assertEqual('gpt', result)
+
 
 class TrySetBootDeviceTestCase(db_base.DbTestCase):
 

ironic/tests/unit/drivers/modules/test_iscsi_deploy.py

@@ -700,38 +700,43 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase):
             mock_image_cache.return_value.clean_up.assert_called_once_with()
             self.assertEqual(uuid_dict_returned, retval)
 
-    def test_get_deploy_info_boot_option_default(self):
+    def _test_get_deploy_info(self, extra_instance_info=None):
+        if extra_instance_info is None:
+            extra_instance_info = {}
+
         instance_info = self.node.instance_info
         instance_info['deploy_key'] = 'key'
+        instance_info.update(extra_instance_info)
         self.node.instance_info = instance_info
         kwargs = {'address': '1.1.1.1', 'iqn': 'target-iqn', 'key': 'key'}
         ret_val = iscsi_deploy.get_deploy_info(self.node, **kwargs)
         self.assertEqual('1.1.1.1', ret_val['address'])
         self.assertEqual('target-iqn', ret_val['iqn'])
+        return ret_val
+
+    def test_get_deploy_info_boot_option_default(self):
+        ret_val = self._test_get_deploy_info()
         self.assertEqual('netboot', ret_val['boot_option'])
 
     def test_get_deploy_info_netboot_specified(self):
-        instance_info = self.node.instance_info
+        capabilities = {'capabilities': {'boot_option': 'netboot'}}
-        instance_info['deploy_key'] = 'key'
+        ret_val = self._test_get_deploy_info(extra_instance_info=capabilities)
-        instance_info['capabilities'] = {'boot_option': 'netboot'}
-        self.node.instance_info = instance_info
-        kwargs = {'address': '1.1.1.1', 'iqn': 'target-iqn', 'key': 'key'}
-        ret_val = iscsi_deploy.get_deploy_info(self.node, **kwargs)
-        self.assertEqual('1.1.1.1', ret_val['address'])
-        self.assertEqual('target-iqn', ret_val['iqn'])
         self.assertEqual('netboot', ret_val['boot_option'])
 
     def test_get_deploy_info_localboot(self):
-        instance_info = self.node.instance_info
+        capabilities = {'capabilities': {'boot_option': 'local'}}
-        instance_info['deploy_key'] = 'key'
+        ret_val = self._test_get_deploy_info(extra_instance_info=capabilities)
-        instance_info['capabilities'] = {'boot_option': 'local'}
-        self.node.instance_info = instance_info
-        kwargs = {'address': '1.1.1.1', 'iqn': 'target-iqn', 'key': 'key'}
-        ret_val = iscsi_deploy.get_deploy_info(self.node, **kwargs)
-        self.assertEqual('1.1.1.1', ret_val['address'])
-        self.assertEqual('target-iqn', ret_val['iqn'])
         self.assertEqual('local', ret_val['boot_option'])
 
+    def test_get_deploy_info_disk_label(self):
+        capabilities = {'capabilities': {'disk_label': 'msdos'}}
+        ret_val = self._test_get_deploy_info(extra_instance_info=capabilities)
+        self.assertEqual('msdos', ret_val['disk_label'])
+
+    def test_get_deploy_info_not_specified(self):
+        ret_val = self._test_get_deploy_info()
+        self.assertNotIn('disk_label', ret_val)
+
     @mock.patch.object(iscsi_deploy, 'continue_deploy', autospec=True)
     @mock.patch.object(iscsi_deploy, 'build_deploy_ramdisk_options',
                        autospec=True)

releasenotes/notes/disk-label-capability-d36d126e0ad36dca.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - Add support for a new capability called 'disk_label' to allow
+    operators to choose the disk label that will be used when Ironic is
+    partitioning the disk.