Follow up to root device hints in instance_info

Follow-up to commit 42dc9787e52670bb1e1baa36f08703dd802804f4:
* Refactor root device checking code to be in one place and extend
  the error message.
* Extend unit tests to cover the actual override.
* Update documentation.

Change-Id: I1d73f2233c766ff52268e242b8071dec12b2daca

9 files changed, 57 insertions, 30 deletions

diff --git a/doc/source/install/include/root-device-hints.inc b/doc/source/install/include/root-device-hints.inc
index af4eef041..76569f3b2 100644
--- a/doc/source/install/include/root-device-hints.inc
+++ b/doc/source/install/include/root-device-hints.inc
@@ -48,6 +48,10 @@ That will guarantee that Bare Metal service will pick the disk device that
 has the ``wwn`` equal to the specified wwn value, or fail the deployment if it
 can not be found.
 
+.. note::
+    Starting with the Ussuri release, root device hints can be specified
+    per-instance, see :doc:`/install/standalone`.
+
 The hints can have an operator at the beginning of the value string. If
 no operator is specified the default is ``==`` (for numerical values)
 and ``s==`` (for string values). The supported operators are:
diff --git a/doc/source/install/standalone.rst b/doc/source/install/standalone.rst
index 04ce182ca..ad645b23e 100644
--- a/doc/source/install/standalone.rst
+++ b/doc/source/install/standalone.rst
@@ -152,6 +152,15 @@ Steps to start a deployment are pretty similar to those when using Compute:
         --instance-info ramdisk=$RAMDISK \
         --instance-info root_gb=10
 
+#. Starting with the Ussuri release, you can set :ref:`root device hints
+   <root-device-hints>` per instance::
+
+    openstack baremetal node set $NODE_UUID \
+        --instance-info root_device='{"wwn": "0x4000cca77fc4dba1"}'
+
+   This setting overrides any previous setting in ``properties`` and will be
+   removed on undeployment.
+
 #. Validate that all parameters are correct::
 
     openstack baremetal node validate $NODE_UUID
diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py
index d42443586..0a2951007 100644
--- a/ironic/drivers/modules/agent.py
+++ b/ironic/drivers/modules/agent.py
@@ -15,7 +15,6 @@
 from urllib import parse as urlparse
 
 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
@@ -431,15 +430,7 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
 
         check_image_size(task, image_source)
         # Validate the root device hints
-        try:
-            root_device = deploy_utils.get_root_device_for_deploy(node)
-            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})
-
+        deploy_utils.get_root_device_for_deploy(node)
         validate_image_proxies(node)
 
     @METRICS.timer('AgentDeploy.deploy')
diff --git a/ironic/drivers/modules/ansible/deploy.py b/ironic/drivers/modules/ansible/deploy.py
index 4682d6f16..f9969936c 100644
--- a/ironic/drivers/modules/ansible/deploy.py
+++ b/ironic/drivers/modules/ansible/deploy.py
@@ -229,15 +229,10 @@ def _parse_partitioning_info(node):
 
 def _parse_root_device_hints(node):
     """Convert string with hints to dict. """
-    root_device = deploy_utils.get_root_device_for_deploy(node)
-    if not root_device:
+    parsed_hints = deploy_utils.get_root_device_for_deploy(node)
+    if not parsed_hints:
         return {}
-    try:
-        parsed_hints = irlib_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})
+
     root_device_hints = {}
     advanced = {}
     for hint, value in parsed_hints.items():
diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index 58133ab9c..3b5c733c7 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -1503,6 +1503,24 @@ def set_async_step_flags(node, reboot=None, skip_current_step=None,
 
 
 def get_root_device_for_deploy(node):
-    """Get a root device requested for deployment or None."""
-    return (node.instance_info.get('root_device')
-            or node.properties.get('root_device'))
+    """Get a root device requested for deployment or None.
+
+    :raises: InvalidParameterValue on invalid hints.
+    :return: Parsed root device hints or None if no hints were provided.
+    """
+    hints = node.instance_info.get('root_device')
+    if not hints:
+        hints = node.properties.get('root_device')
+        if not hints:
+            return
+        source = 'properties'
+    else:
+        source = 'instance_info'
+
+    try:
+        return il_utils.parse_root_device_hints(hints)
+    except ValueError as e:
+        raise exception.InvalidParameterValue(
+            _('Failed to validate the root device hints %(hints)s (from the '
+              'node\'s %(source)s) for node %(node)s. Error: %(error)s') %
+            {'node': node.uuid, 'hints': hints, 'source': source, 'error': e})
diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py
index 0417e3f47..cce6704c8 100644
--- a/ironic/drivers/modules/iscsi_deploy.py
+++ b/ironic/drivers/modules/iscsi_deploy.py
@@ -17,7 +17,6 @@ from urllib import parse as urlparse
 
 from ironic_lib import disk_utils
 from ironic_lib import metrics_utils
-from ironic_lib import utils as il_utils
 from oslo_log import log as logging
 from oslo_utils import excutils
 
@@ -320,14 +319,7 @@ def validate(task):
     # TODO(lucasagomes): Validate the format of the URL
     deploy_utils.get_ironic_api_url()
     # Validate the root device hints
-    try:
-        root_device = deploy_utils.get_root_device_for_deploy(task.node)
-        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.get_root_device_for_deploy(task.node)
     deploy_utils.parse_instance_info(task.node)
 
 
diff --git a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
index 981f673b1..8787848ff 100644
--- a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
+++ b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
@@ -369,6 +369,22 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase):
             self.assertEqual(
                 expected, ansible_deploy._parse_root_device_hints(task.node))
 
+    def test__parse_root_device_hints_override(self):
+        hints = {"wwn": "fake wwn", "size": "12345", "rotational": True,
+                 "serial": "HELLO"}
+        expected = {"wwn": "fake wwn", "size": 12345, "rotational": True,
+                    "serial": "hello"}
+        props = self.node.properties
+        props['root_device'] = {'size': 'no idea'}
+        self.node.properties = props
+        iinfo = self.node.instance_info
+        iinfo['root_device'] = hints
+        self.node.instance_info = iinfo
+        self.node.save()
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            self.assertEqual(
+                expected, ansible_deploy._parse_root_device_hints(task.node))
+
     def test__parse_root_device_hints_fail_advanced(self):
         hints = {"wwn": "s!= fake wwn",
                  "size": ">= 12345",
diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py
index 03deafcc7..7ec76c9d0 100644
--- a/ironic/tests/unit/drivers/modules/test_agent.py
+++ b/ironic/tests/unit/drivers/modules/test_agent.py
@@ -281,6 +281,7 @@ class TestAgentDeploy(db_base.DbTestCase):
             self, pxe_boot_validate_mock, show_mock, validate_http_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
+            task.node.properties['root_device'] = {'size': 42}
             task.node.instance_info['root_device'] = {'size': 'not-int'}
             self.assertRaises(exception.InvalidParameterValue,
                               task.driver.deploy.validate, task)
diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
index 9d2b42609..a5fee72a5 100644
--- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
+++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
@@ -579,6 +579,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase):
         mock_get_url.return_value = 'http://spam.ham/baremetal'
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
+            task.node.properties['root_device'] = {'size': 42}
             task.node.instance_info['root_device'] = {'size': 'not-int'}
             self.assertRaises(exception.InvalidParameterValue,
                               iscsi_deploy.validate, task)