Allow diskless nodes introspection

There is a demand to use introspection on diskless nodes to figure out
what is possible to figure out.

We might need more changes to properly support diskless nodes, this
change is just to allow people to play with it.

The property ``local_gb == 0`` for a diskless node.

Change-Id: I21b2f2c069fdbf767367ec3d1fbf77bab6292b25
Partial-Bug: #1554243

doc/source/usage.rst

@@ -187,6 +187,13 @@ unless you understand what you're doing:
 ``scheduler``
     validates and updates basic hardware scheduling properties: CPU number and
     architecture, memory and disk size.
+
+    .. note::
+
+      Diskless nodes have the disk size property ``local_gb == 0``. Always use
+      node driver ``root_device`` hints to prevent unexpected HW failures
+      passing silently.
+
 ``validate_interfaces``
     validates network interfaces information.
 

ironic_inspector/plugins/standard.py

@@ -88,8 +88,7 @@ class SchedulerHook(base.ProcessingHook):
             if CONF.processing.disk_partitioning_spacing:
                 introspection_data['local_gb'] -= 1
         else:
-            errors.append(_('root disk is not supplied by the ramdisk and '
-                            'root_disk_selection hook is not enabled'))
+            introspection_data['local_gb'] = 0
 
         try:
             introspection_data['cpus'] = int(inventory['cpu']['count'])

ironic_inspector/test/functional.py

@@ -687,6 +687,32 @@ class Test(Base):
         self.assertIsNone(row.error)
         self.assertNotEqual(version_id, row.version_id)
 
+    def test_without_root_disk(self):
+        del self.data['root_disk']
+        self.inventory['disks'] = []
+        self.patch[-1] = {'path': '/properties/local_gb',
+                          'value': '0', 'op': 'add'}
+
+        self.call_introspect(self.uuid)
+        eventlet.greenthread.sleep(DEFAULT_SLEEP)
+        self.cli.node.set_power_state.assert_called_once_with(self.uuid,
+                                                              'reboot')
+
+        status = self.call_get_status(self.uuid)
+        self.check_status(status, finished=False)
+
+        res = self.call_continue(self.data)
+        self.assertEqual({'uuid': self.uuid}, res)
+        eventlet.greenthread.sleep(DEFAULT_SLEEP)
+
+        self.cli.node.update.assert_called_once_with(self.uuid, mock.ANY)
+        self.assertCalledWithPatch(self.patch, self.cli.node.update)
+        self.cli.port.create.assert_called_once_with(
+            node_uuid=self.uuid, address='11:22:33:44:55:66')
+
+        status = self.call_get_status(self.uuid)
+        self.check_status(status, finished=True)
+
 
 @contextlib.contextmanager
 def mocked_server():

ironic_inspector/test/unit/test_plugins_standard.py

@@ -12,6 +12,8 @@
 # limitations under the License.
 
 import mock
+import six
+
 from oslo_config import cfg
 from oslo_utils import units
 
@@ -37,11 +39,21 @@ class TestSchedulerHook(test_base.NodeTest):
         ext = base.processing_hooks_manager()['scheduler']
         self.assertIsInstance(ext.obj, std_plugins.SchedulerHook)
 
-    def test_no_root_disk(self):
+    @mock.patch.object(node_cache.NodeInfo, 'patch')
+    def test_no_root_disk(self, mock_patch):
         del self.inventory['disks']
-        self.assertRaisesRegex(utils.Error, 'disks key is missing or empty',
-                               self.hook.before_update, self.data,
-                               self.node_info)
+        del self.data['root_disk']
+
+        patch = [
+            {'path': '/properties/cpus', 'value': '4', 'op': 'add'},
+            {'path': '/properties/cpu_arch', 'value': 'x86_64', 'op': 'add'},
+            {'path': '/properties/memory_mb', 'value': '12288', 'op': 'add'},
+            {'path': '/properties/local_gb', 'value': '0', 'op': 'add'}
+        ]
+
+        self.hook.before_update(self.data, self.node_info)
+        self.assertCalledWithPatch(patch, mock_patch)
+        self.assertEqual(0, self.data['local_gb'])
 
     @mock.patch.object(node_cache.NodeInfo, 'patch')
     def test_ok(self, mock_patch):
@@ -265,10 +277,10 @@ class TestRootDiskSelection(test_base.NodeTest):
         self.node.properties['root_device'] = {'size': 10}
         self.inventory['disks'] = []
 
-        self.assertRaisesRegex(utils.Error,
-                               'disks key is missing or empty',
-                               self.hook.before_update,
-                               self.data, self.node_info)
+        six.assertRaisesRegex(self, utils.Error,
+                              'No disks satisfied root device hints',
+                              self.hook.before_update,
+                              self.data, self.node_info)
 
     def test_one_matches(self):
         self.node.properties['root_device'] = {'size': 10}

ironic_inspector/utils.py

@@ -22,7 +22,7 @@ from oslo_middleware import cors as cors_middleware
 import pytz
 
 from ironicclient.v1 import node
-from ironic_inspector.common.i18n import _, _LE
+from ironic_inspector.common.i18n import _, _LE, _LI
 from ironic_inspector import conf  # noqa
 
 CONF = cfg.CONF
@@ -217,7 +217,7 @@ def get_valid_macs(data):
             if m.get('mac')]
 
 
-_INVENTORY_MANDATORY_KEYS = ('disks', 'memory', 'cpu', 'interfaces')
+_INVENTORY_MANDATORY_KEYS = ('memory', 'cpu', 'interfaces')
 
 
 def get_inventory(data, node_info=None):
@@ -233,6 +233,12 @@ def get_inventory(data, node_info=None):
             raise Error(_('Invalid hardware inventory: %s key is missing '
                           'or empty') % key, data=data, node_info=node_info)
 
+    if not inventory.get('disks'):
+        LOG.info(_LI('No disks were detected in the inventory, assuming this '
+                     'is a disk-less node'), data=data, node_info=node_info)
+        # Make sure the code iterating over it does not fail with a TypeError
+        inventory['disks'] = []
+
     return inventory
 
 

releasenotes/notes/optional-root-disk-9b972f504b2e6262.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+   Avoid failing introspection on diskless nodes. The node property ``local_gb
+   == 0`` is set in that case.