Replace parse_root_device_hints with the ironic-lib version one

This patch is replacing the parse_root_device_hints() method from
deploy_utils.py with the ironic-lib version.

The parse_root_device_hints() method is only being used to verify the
syntax of the root device hints, finding/matching a disk with the hints
is performed by IPA.

The ironic-lib version also does add support for using operators such as
(>=, >, <, etc...) to finding a suitable disk device.

Closes-Bug: #1561137
Depends-On: Id93dd0360137df600f5a656348279e56c6b84bf9
Change-Id: I4516d9f1f1733be26ae603b70c1e59ccf19e0448

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)

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.
 

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)
 
 

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',

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<``, ``<in>``, ``<all-in>`` and ``<or>``.