From 6f36a5e48739842c0e7c544cfcaaf84392dbf855 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Tue, 23 Feb 2016 10:36:19 +0000 Subject: [PATCH] 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 | 20 +++++++++- ironic/drivers/modules/iscsi_deploy.py | 5 +++ .../unit/drivers/modules/test_deploy_utils.py | 20 ++++++++-- .../unit/drivers/modules/test_iscsi_deploy.py | 39 +++++++++++-------- ...isk-label-capability-d36d126e0ad36dca.yaml | 5 +++ 5 files changed, 67 insertions(+), 22 deletions(-) create mode 100644 releasenotes/notes/disk-label-capability-d36d126e0ad36dca.yaml diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 48d1a2c7c8..88c69fc423 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/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. diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index cdd68d9f61..a7a464830f 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/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( diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 9fee720439..8a916b11ac 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/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): diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index 1806fe70b3..e6644bda89 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/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 - instance_info['deploy_key'] = 'key' - 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']) + capabilities = {'capabilities': {'boot_option': 'netboot'}} + ret_val = self._test_get_deploy_info(extra_instance_info=capabilities) self.assertEqual('netboot', ret_val['boot_option']) def test_get_deploy_info_localboot(self): - instance_info = self.node.instance_info - instance_info['deploy_key'] = 'key' - 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']) + capabilities = {'capabilities': {'boot_option': 'local'}} + ret_val = self._test_get_deploy_info(extra_instance_info=capabilities) 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) diff --git a/releasenotes/notes/disk-label-capability-d36d126e0ad36dca.yaml b/releasenotes/notes/disk-label-capability-d36d126e0ad36dca.yaml new file mode 100644 index 0000000000..95365283a7 --- /dev/null +++ b/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.