From 42dc9787e52670bb1e1baa36f08703dd802804f4 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Fri, 3 Jan 2020 17:59:03 +0100
Subject: [PATCH] Allow reading root_device from instance_info

For the future deployment API we need to be able to set root_device
per deployment in addition to per node. This change adds it.

Change-Id: I1dd046c2e5fca211a84290bac8daa7550b21614f
Depends-On: https://review.opendev.org/701043
Story: #2006910
Task: #37556
---
 ironic/drivers/modules/agent.py                  |  2 +-
 ironic/drivers/modules/ansible/deploy.py         |  2 +-
 ironic/drivers/modules/deploy_utils.py           |  6 ++++++
 ironic/drivers/modules/iscsi_deploy.py           |  2 +-
 .../unit/drivers/modules/ansible/test_deploy.py  | 13 +++++++++++++
 ironic/tests/unit/drivers/modules/test_agent.py  | 16 ++++++++++++++++
 .../unit/drivers/modules/test_iscsi_deploy.py    |  9 +++++++++
 ...stance-info-root-device-0a5190240fcc8fd8.yaml |  6 ++++++
 8 files changed, 53 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/instance-info-root-device-0a5190240fcc8fd8.yaml

diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py
index 5e7d18e04d..d42443586b 100644
--- a/ironic/drivers/modules/agent.py
+++ b/ironic/drivers/modules/agent.py
@@ -432,7 +432,7 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
         check_image_size(task, image_source)
         # Validate the root device hints
         try:
-            root_device = node.properties.get('root_device')
+            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(
diff --git a/ironic/drivers/modules/ansible/deploy.py b/ironic/drivers/modules/ansible/deploy.py
index 51d7f7b04f..4682d6f165 100644
--- a/ironic/drivers/modules/ansible/deploy.py
+++ b/ironic/drivers/modules/ansible/deploy.py
@@ -229,7 +229,7 @@ def _parse_partitioning_info(node):
 
 def _parse_root_device_hints(node):
     """Convert string with hints to dict. """
-    root_device = node.properties.get('root_device')
+    root_device = deploy_utils.get_root_device_for_deploy(node)
     if not root_device:
         return {}
     try:
diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index f8c9728539..51fa0c708e 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -1509,3 +1509,9 @@ def set_async_step_flags(node, reboot=None, skip_current_step=None,
         info[fields['polling']] = polling
     node.driver_internal_info = info
     node.save()
+
+
+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'))
diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py
index 89603a8e6b..0417e3f475 100644
--- a/ironic/drivers/modules/iscsi_deploy.py
+++ b/ironic/drivers/modules/iscsi_deploy.py
@@ -321,7 +321,7 @@ def validate(task):
     deploy_utils.get_ironic_api_url()
     # Validate the root device hints
     try:
-        root_device = task.node.properties.get('root_device')
+        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(
diff --git a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
index 9cf759af45..981f673b17 100644
--- a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
+++ b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
@@ -356,6 +356,19 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase):
             self.assertEqual(
                 expected, ansible_deploy._parse_root_device_hints(task.node))
 
+    def test__parse_root_device_hints_iinfo(self):
+        hints = {"wwn": "fake wwn", "size": "12345", "rotational": True,
+                 "serial": "HELLO"}
+        expected = {"wwn": "fake wwn", "size": 12345, "rotational": True,
+                    "serial": "hello"}
+        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 15d5b675a2..03deafcc7c 100644
--- a/ironic/tests/unit/drivers/modules/test_agent.py
+++ b/ironic/tests/unit/drivers/modules/test_agent.py
@@ -273,6 +273,22 @@ class TestAgentDeploy(db_base.DbTestCase):
             show_mock.assert_called_once_with(self.context, 'fake-image')
             validate_http_mock.assert_called_once_with(task.node)
 
+    @mock.patch.object(agent, 'validate_http_provisioning_configuration',
+                       autospec=True)
+    @mock.patch.object(images, 'image_show', autospec=True)
+    @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True)
+    def test_validate_invalid_root_device_hints_iinfo(
+            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.instance_info['root_device'] = {'size': 'not-int'}
+            self.assertRaises(exception.InvalidParameterValue,
+                              task.driver.deploy.validate, task)
+            pxe_boot_validate_mock.assert_called_once_with(
+                task.driver.boot, task)
+            show_mock.assert_called_once_with(self.context, 'fake-image')
+            validate_http_mock.assert_called_once_with(task.node)
+
     @mock.patch.object(agent, 'validate_http_provisioning_configuration',
                        autospec=True)
     @mock.patch.object(images, 'image_show', autospec=True)
diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
index 36192ea3ed..9d2b42609b 100644
--- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
+++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
@@ -574,6 +574,15 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase):
             self.assertRaises(exception.InvalidParameterValue,
                               iscsi_deploy.validate, task)
 
+    @mock.patch('ironic.drivers.modules.deploy_utils.get_ironic_api_url')
+    def test_validate_invalid_root_device_hints_iinfo(self, mock_get_url):
+        mock_get_url.return_value = 'http://spam.ham/baremetal'
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            task.node.instance_info['root_device'] = {'size': 'not-int'}
+            self.assertRaises(exception.InvalidParameterValue,
+                              iscsi_deploy.validate, task)
+
 
 class ISCSIDeployTestCase(db_base.DbTestCase):
 
diff --git a/releasenotes/notes/instance-info-root-device-0a5190240fcc8fd8.yaml b/releasenotes/notes/instance-info-root-device-0a5190240fcc8fd8.yaml
new file mode 100644
index 0000000000..e9281b20f0
--- /dev/null
+++ b/releasenotes/notes/instance-info-root-device-0a5190240fcc8fd8.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Allows reading the ``root_device`` from ``instance_info``, overriding
+    the value in ``properties``. This enables per-instance root device
+    settings and requires the Ussuri release of ironic-python-agent.