diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index eda5e6f175..2c8e09aad2 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -13,6 +13,7 @@ # limitations under the License. from ironic_lib import metrics_utils +from ironic_lib import utils as il_utils from oslo_log import log from oslo_utils import excutils from oslo_utils import units @@ -394,7 +395,14 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): check_image_size(task, image_source) # Validate the root device hints - deploy_utils.parse_root_device_hints(node) + try: + root_device = node.properties.get('root_device') + il_utils.parse_root_device_hints(root_device) + except ValueError as e: + raise exception.InvalidParameterValue( + _('Failed to validate the root device hints for node ' + '%(node)s. Error: %(error)s') % {'node': node.uuid, + 'error': e}) # Validate node capabilities deploy_utils.validate_capabilities(node) diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 38b2439bf5..5c96e0ed6c 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -26,7 +26,6 @@ from oslo_serialization import jsonutils from oslo_utils import excutils from oslo_utils import strutils import six -from six.moves.urllib import parse from ironic.common import dhcp_factory from ironic.common import exception @@ -691,60 +690,6 @@ def try_set_boot_device(task, device, persistent=True): raise -def parse_root_device_hints(node): - """Parse the root_device property of a node. - - Parse the root_device property of a node and make it a flat string - to be passed via the PXE config. - - :param node: a single Node. - :returns: A flat string with the following format - opt1=value1,opt2=value2. Or None if the - Node contains no hints. - :raises: InvalidParameterValue, if some information is invalid. - - """ - root_device = node.properties.get('root_device') - if not root_device: - return - - # Find invalid hints for logging - invalid_hints = set(root_device) - VALID_ROOT_DEVICE_HINTS - if invalid_hints: - raise exception.InvalidParameterValue( - _('The hints "%(invalid_hints)s" are invalid. ' - 'Valid hints are: "%(valid_hints)s"') % - {'invalid_hints': ', '.join(invalid_hints), - 'valid_hints': ', '.join(VALID_ROOT_DEVICE_HINTS)}) - - if 'size' in root_device: - try: - int(root_device['size']) - except ValueError: - raise exception.InvalidParameterValue( - _('Root device hint "size" is not an integer value.')) - - if 'rotational' in root_device: - try: - strutils.bool_from_string(root_device['rotational'], strict=True) - except ValueError: - raise exception.InvalidParameterValue( - _('Root device hint "rotational" is not a boolean value.')) - - hints = [] - for key, value in sorted(root_device.items()): - # NOTE(lucasagomes): We can't have spaces in the PXE config - # file, so we are going to url/percent encode the value here - # and decode on the other end. - if isinstance(value, six.string_types): - value = value.strip() - value = parse.quote(value) - - hints.append("%s=%s" % (key, value)) - - return ','.join(hints) - - def is_secure_boot_requested(node): """Returns True if secure_boot is requested for deploy. diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 74d67f8967..09e0b280b7 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -17,7 +17,7 @@ import os from ironic_lib import disk_utils from ironic_lib import metrics_utils -from ironic_lib import utils as ironic_utils +from ironic_lib import utils as il_utils from oslo_log import log as logging from oslo_utils import fileutils from six.moves.urllib import parse @@ -134,7 +134,7 @@ def destroy_images(node_uuid): :param node_uuid: the uuid of the ironic node. """ - ironic_utils.unlink_without_raise(_get_image_file_path(node_uuid)) + il_utils.unlink_without_raise(_get_image_file_path(node_uuid)) utils.rmtree_without_raise(_get_image_dir_path(node_uuid)) InstanceImageCache().clean_up() @@ -353,7 +353,14 @@ def validate(task): # TODO(lucasagomes): Validate the format of the URL deploy_utils.get_ironic_api_url() # Validate the root device hints - deploy_utils.parse_root_device_hints(task.node) + try: + root_device = task.node.properties.get('root_device') + il_utils.parse_root_device_hints(root_device) + except ValueError as e: + raise exception.InvalidParameterValue( + _('Failed to validate the root device hints for node ' + '%(node)s. Error: %(error)s') % {'node': task.node.uuid, + 'error': e}) deploy_utils.parse_instance_info(task.node) diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 48b6081efe..ff4aeef26e 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -1383,45 +1383,6 @@ class OtherFunctionTestCase(db_base.DbTestCase): actual = utils.get_dev('1.2.3.4', 5678, 'iqn.fake', 9) self.assertEqual(expected, actual) - def test_parse_root_device_hints(self): - self.node.properties['root_device'] = { - 'wwn': '123456', 'model': 'foo-model', 'size': 123, - 'serial': 'foo-serial', 'vendor': 'foo-vendor', 'name': '/dev/sda', - 'wwn_with_extension': '123456111', 'wwn_vendor_extension': '111', - 'rotational': True, - } - expected = ('model=foo-model,name=/dev/sda,rotational=True,' - 'serial=foo-serial,size=123,vendor=foo-vendor,wwn=123456,' - 'wwn_vendor_extension=111,wwn_with_extension=123456111') - result = utils.parse_root_device_hints(self.node) - self.assertEqual(expected, result) - - def test_parse_root_device_hints_string_space(self): - self.node.properties['root_device'] = {'model': 'fake model'} - expected = 'model=fake%20model' - result = utils.parse_root_device_hints(self.node) - self.assertEqual(expected, result) - - def test_parse_root_device_hints_no_hints(self): - self.node.properties = {} - result = utils.parse_root_device_hints(self.node) - self.assertIsNone(result) - - def test_parse_root_device_hints_invalid_hints(self): - self.node.properties['root_device'] = {'vehicle': 'Owlship'} - self.assertRaises(exception.InvalidParameterValue, - utils.parse_root_device_hints, self.node) - - def test_parse_root_device_hints_invalid_size(self): - self.node.properties['root_device'] = {'size': 'not-int'} - self.assertRaises(exception.InvalidParameterValue, - utils.parse_root_device_hints, self.node) - - def test_parse_root_device_hints_invalid_rotational(self): - self.node.properties['root_device'] = {'rotational': 'not-boolean'} - self.assertRaises(exception.InvalidParameterValue, - utils.parse_root_device_hints, self.node) - @mock.patch.object(utils, 'LOG', autospec=True) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(task_manager.TaskManager, 'process_event', diff --git a/releasenotes/notes/support-root-device-hints-with-operators-96cf34fa37b5b2e8.yaml b/releasenotes/notes/support-root-device-hints-with-operators-96cf34fa37b5b2e8.yaml new file mode 100644 index 0000000000..56ee861714 --- /dev/null +++ b/releasenotes/notes/support-root-device-hints-with-operators-96cf34fa37b5b2e8.yaml @@ -0,0 +1,6 @@ +--- +features: + - Adds support for using operators with the root device hints mechanism. + The currently supported operators are, ``=``, ``==``, ``!=``, ``>=``, + ``<=``, ``>``, ``<``, ``s==``, ``s!=``, ``s>=``, ``s>``, ``s<=``, + ``s<``, ````, ```` and ````.