From 170ba4f29520b338f8b54b1225541db974b40a86 Mon Sep 17 00:00:00 2001
From: Lucas Alvares Gomes <lucasagomes@gmail.com>
Date: Wed, 6 Jul 2016 14:42:31 +0100
Subject: [PATCH] Add and document the "rotational" root device hint

This patch is adding a new root device hint called "rotational". This
hint is used to identify whether a device is rotational or not making it
easy to distinguish HDDs and SSDs when choosing which disk Ironic
should deploy the image onto.

Closes-Bug: #1599517
Depends-On: I270fe57df825929bdef7911b3a6757cf7163a5f1
Change-Id: Id630a0b9d02ed8e1bd674c32bef0d489849c3f29
---
 doc/source/deploy/install-guide.rst              |  3 +++
 ironic/drivers/modules/deploy_utils.py           |  9 ++++++++-
 .../unit/drivers/modules/test_deploy_utils.py    | 16 +++++++++++-----
 ...device-hints-rotational-c21f02130394e1d4.yaml |  4 ++++
 4 files changed, 26 insertions(+), 6 deletions(-)
 create mode 100644 releasenotes/notes/root-device-hints-rotational-c21f02130394e1d4.yaml

diff --git a/doc/source/deploy/install-guide.rst b/doc/source/deploy/install-guide.rst
index 6ded72df16..8cfdd85f80 100644
--- a/doc/source/deploy/install-guide.rst
+++ b/doc/source/deploy/install-guide.rst
@@ -1905,6 +1905,9 @@ deployment. The list of support hints is:
 * wwn (STRING): unique storage identifier
 * wwn_with_extension (STRING): unique storage identifier with the vendor extension appended
 * wwn_vendor_extension (STRING): unique vendor storage identifier
+* rotational (BOOLEAN): whether it's a rotational device or not. This
+  hint makes it easier to distinguish HDDs (rotational) and SSDs (not
+  rotational) when choosing which disk Ironic should deploy the image onto.
 * name (STRING): the device name, e.g /dev/md0
 
 
diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index b37bec11a9..2914463c55 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -104,7 +104,7 @@ LOG = logging.getLogger(__name__)
 
 VALID_ROOT_DEVICE_HINTS = set(('size', 'model', 'wwn', 'serial', 'vendor',
                                'wwn_with_extension', 'wwn_vendor_extension',
-                               'name'))
+                               'name', 'rotational'))
 
 SUPPORTED_CAPABILITIES = {
     'boot_option': ('local', 'netboot'),
@@ -710,6 +710,13 @@ def parse_root_device_hints(node):
             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
diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
index bb362b7c40..ef09b252c6 100644
--- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
@@ -1214,13 +1214,14 @@ class OtherFunctionTestCase(db_base.DbTestCase):
 
     def test_parse_root_device_hints(self):
         self.node.properties['root_device'] = {
-            'wwn': 123456, 'model': 'foo-model', 'size': 123,
+            'wwn': '123456', 'model': 'foo-model', 'size': 123,
             'serial': 'foo-serial', 'vendor': 'foo-vendor', 'name': '/dev/sda',
-            'wwn_with_extension': 123456111, 'wwn_vendor_extension': 111,
+            'wwn_with_extension': '123456111', 'wwn_vendor_extension': '111',
+            'rotational': True,
         }
-        expected = ('model=foo-model,name=/dev/sda,serial=foo-serial,size=123,'
-                    'vendor=foo-vendor,wwn=123456,wwn_vendor_extension=111,'
-                    'wwn_with_extension=123456111')
+        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)
 
@@ -1245,6 +1246,11 @@ class OtherFunctionTestCase(db_base.DbTestCase):
         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/root-device-hints-rotational-c21f02130394e1d4.yaml b/releasenotes/notes/root-device-hints-rotational-c21f02130394e1d4.yaml
new file mode 100644
index 0000000000..000fce1093
--- /dev/null
+++ b/releasenotes/notes/root-device-hints-rotational-c21f02130394e1d4.yaml
@@ -0,0 +1,4 @@
+---
+features:
+  - Extend the root device hints to identify whether a disk is rotational
+    or not.