Add module for in-band inspection using ironic-discoverd

Also adds FakeDiscoverdDriver for the sake of testing.

Change-Id: I49463e7eb897c73b42e1d2816b4f2001b885c7f6
Implements: blueprint inband-properties-discovery

driver-requirements.txt

@@ -4,6 +4,7 @@
 # python projects they should package as optional dependencies for Ironic.
 
 # These are available on pypi
+ironic-discoverd>=1.0.0
 proliantutils>=2.0.1
 pyghmi
 pysnmp

etc/ironic/ironic.conf.sample

@@ -15,7 +15,7 @@
 #rpc_conn_pool_size=30
 
 # Qpid broker hostname. (string value)
-#qpid_hostname=ironic
+#qpid_hostname=localhost
 
 # Qpid broker port. (integer value)
 #qpid_port=5672
@@ -76,7 +76,7 @@
 
 # The RabbitMQ broker address where a single node is used.
 # (string value)
-#rabbit_host=ironic
+#rabbit_host=localhost
 
 # The RabbitMQ broker port where a single node is used.
 # (integer value)
@@ -781,6 +781,26 @@
 #dhcp_provider=neutron
 
 
+[discoverd]
+
+#
+# Options defined in ironic.drivers.modules.discoverd
+#
+
+# whether to enable inspection using ironic-discoverd (boolean
+# value)
+#enabled=false
+
+# ironic-discoverd HTTP endpoint. If this is not set, the
+# ironic-discoverd client default (http://127.0.0.1:5050) will
+# be used. (string value)
+#service_url=<None>
+
+# period (in seconds) to check status of nodes on inspection
+# (integer value)
+#status_check_period=60
+
+
 [disk_partitioner]
 
 #

ironic/api/controllers/v1/node.py

@@ -1012,10 +1012,11 @@ class NodesController(rest.RestController):
 
         rpc_node = _get_rpc_node(node_ident)
 
-        # Check if node is transitioning state, although nodes in DEPLOYFAIL
+        # Check if node is transitioning state, although nodes in some states
         # can be updated.
         if ((rpc_node.target_power_state or rpc_node.target_provision_state)
-                and rpc_node.provision_state != ir_states.DEPLOYFAIL):
+                and rpc_node.provision_state not in
+                ir_states.UPDATE_ALLOWED_STATES):
             msg = _("Node %s can not be updated while a state transition "
                     "is in progress.")
             raise wsme.exc.ClientSideError(msg % node_ident, status_code=409)

ironic/common/states.py

@@ -149,6 +149,11 @@ inspected node shall transition to MANAGEABLE status.
 INSPECTFAIL = 'inspect failed'
 """ Node inspection failed. """
 
+
+UPDATE_ALLOWED_STATES = (DEPLOYFAIL, INSPECTING, INSPECTFAIL)
+"""Transitional states in which we allow updating a node."""
+
+
 ##############
 # Power states
 ##############

ironic/drivers/fake.py

@@ -23,6 +23,7 @@ from ironic.common import exception
 from ironic.common.i18n import _
 from ironic.drivers import base
 from ironic.drivers.modules import agent
+from ironic.drivers.modules import discoverd
 from ironic.drivers.modules.drac import management as drac_mgmt
 from ironic.drivers.modules.drac import power as drac_power
 from ironic.drivers.modules import fake
@@ -201,3 +202,15 @@ class FakeVirtualBoxDriver(base.BaseDriver):
         self.power = virtualbox.VirtualBoxPower()
         self.deploy = fake.FakeDeploy()
         self.management = virtualbox.VirtualBoxManagement()
+
+
+class FakeIPMIToolDiscoverdDriver(base.BaseDriver):
+    """Fake Discoverd driver."""
+
+    def __init__(self):
+        self.power = ipmitool.IPMIPower()
+        self.console = ipmitool.IPMIShellinaboxConsole()
+        self.deploy = fake.FakeDeploy()
+        self.vendor = ipmitool.VendorPassthru()
+        self.management = ipmitool.IPMIManagement()
+        self.inspect = discoverd.DiscoverdInspect()

ironic/drivers/modules/discoverd.py

@@ -0,0 +1,193 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+"""
+Modules required to work with ironic_discoverd:
+    https://pypi.python.org/pypi/ironic-discoverd
+"""
+
+import eventlet
+from oslo_config import cfg
+from oslo_utils import importutils
+
+from ironic.common import exception
+from ironic.common.i18n import _
+from ironic.common.i18n import _LE
+from ironic.common.i18n import _LI
+from ironic.common import keystone
+from ironic.common import states
+from ironic.conductor import task_manager
+from ironic.drivers import base
+from ironic.openstack.common import log as logging
+
+
+LOG = logging.getLogger(__name__)
+
+
+discoverd_opts = [
+    cfg.BoolOpt('enabled', default=False,
+                help='whether to enable inspection using ironic-discoverd'),
+    cfg.StrOpt('service_url',
+               help='ironic-discoverd HTTP endpoint. If this is not set, the '
+               'ironic-discoverd client default (http://127.0.0.1:5050) '
+               'will be used.'),
+    cfg.IntOpt('status_check_period', default=60,
+               help='period (in seconds) to check status of nodes '
+               'on inspection')
+]
+
+CONF = cfg.CONF
+CONF.register_opts(discoverd_opts, group='discoverd')
+
+
+ironic_discoverd = importutils.try_import('ironic_discoverd')
+if ironic_discoverd:
+    from ironic_discoverd import client
+
+
+class DiscoverdInspect(base.InspectInterface):
+    """In-band inspection via ironic-discoverd project."""
+
+    def __init__(self):
+        if not CONF.discoverd.enabled:
+            LOG.info('ironic-discoverd support is disabled')
+            return
+
+        if not ironic_discoverd:
+            raise exception.DriverLoadError(
+                _('ironic-discoverd Python module not found'))
+
+        # NOTE(dtantsur): __version_info__ attribute appeared in 1.0.0
+        version = getattr(ironic_discoverd, '__version_info__', (0, 2))
+        if version < (1, 0):
+            raise exception.DriverLoadError(
+                _('ironic-discoverd version is too old: required >= 1.0.0 '
+                  'got %s') % '.'.join(str(x) for x in version))
+
+    def get_properties(self):
+        """Return the properties of the interface.
+
+        :returns: dictionary of <property name>:<property description> entries.
+        """
+        return {}  # no properties
+
+    def validate(self, task):
+        """Validate the driver-specific management information.
+
+        If invalid, raises an exception; otherwise returns None.
+
+        :param task: a task from TaskManager.
+        :raises: UnsupportedDriverExtension if discoverd support is disabled
+        """
+        if not CONF.discoverd.enabled:
+            raise exception.UnsupportedDriverExtension(
+                _('ironic-discoverd support is disabled in '
+                  'configuration, set [discoverd]enabled to true '
+                  'to enable'))
+
+    def inspect_hardware(self, task):
+        """Inspect hardware to obtain the hardware properties.
+
+        This particular implementation only starts inspection using
+        ironic-discoverd. Results will be checked in a periodic task.
+
+        :param task: a task from TaskManager.
+        :returns: states.INSPECTING
+        """
+        LOG.debug('Starting inspection for node %(uuid)s using '
+                  'ironic-discoverd client %(version)s',
+                  {'uuid': task.node.uuid, 'version':
+                   ironic_discoverd.__version__})
+
+        # NOTE(dtantsur): we're spawning a short-living green thread so that
+        # we can release a lock as soon as possible and allow ironic-discoverd
+        # to operate on a node.
+        eventlet.spawn_n(_start_inspection, task.node.uuid, task.context)
+        return states.INSPECTING
+
+    @base.driver_periodic_task(spacing=CONF.discoverd.status_check_period,
+                               enabled=CONF.discoverd.enabled)
+    def _periodic_check_result(self, manager, context):
+        """Periodic task checking results of inspection."""
+        filters = {'provision_state': states.INSPECTING}
+        node_iter = manager.iter_nodes(filters=filters)
+
+        for node_uuid, driver in node_iter:
+            try:
+                # TODO(dtantsur): we need an exclusive lock only once
+                # inspection is finished.
+                with task_manager.acquire(context, node_uuid) as task:
+                    _check_status(task)
+            except (exception.NodeLocked, exception.NodeNotFound):
+                continue
+
+
+def _call_discoverd(func, uuid, context):
+    """Wrapper around calls to discoverd."""
+    # NOTE(dtantsur): due to bug #1428652 None is not accepted for base_url.
+    kwargs = {}
+    if CONF.discoverd.service_url:
+        kwargs['base_url'] = CONF.discoverd.service_url
+    return func(uuid, auth_token=context.auth_token, **kwargs)
+
+
+def _start_inspection(node_uuid, context):
+    """Call to discoverd to start inspection."""
+    try:
+        _call_discoverd(client.introspect, node_uuid, context)
+    except Exception as exc:
+        LOG.exception(_LE('Exception during contacting ironic-discoverd '
+                          'for inspection of node %(node)s: %(err)s'),
+                      {'node': node_uuid, 'err': exc})
+        # NOTE(dtantsur): if acquire fails our last option is to rely on
+        # timeout
+        with task_manager.acquire(context, node_uuid) as task:
+            task.node.last_error = _('Failed to start inspection: %s') % exc
+            task.process_event('fail')
+    else:
+        LOG.info(_LI('Node %s was sent to inspection to ironic-discoverd'),
+                 node_uuid)
+
+
+def _check_status(task):
+    """Check inspection status for node given by a task."""
+    node = task.node
+    if node.provision_state != states.INSPECTING:
+        return
+    if not isinstance(task.driver.inspect, DiscoverdInspect):
+        return
+
+    LOG.debug('Calling to discoverd to check status of node %s',
+              task.node.uuid)
+
+    # NOTE(dtantsur): periodic tasks do not have proper tokens in context
+    task.context.auth_token = keystone.get_admin_auth_token()
+    try:
+        status = _call_discoverd(client.get_status, node.uuid, task.context)
+    except Exception:
+        # NOTE(dtantsur): get_status should not normally raise
+        # let's assume it's a transient failure and retry later
+        LOG.exception(_LE('Unexpected exception while getting '
+                          'inspection status for node %s, will retry later'),
+                      node.uuid)
+        return
+
+    if status.get('error'):
+        LOG.error(_LE('Inspection failed for node %(uuid)s '
+                      'with error: %(err)s'),
+                  {'uuid': node.uuid, 'err': status['error']})
+        node.last_error = _('ironic-discoverd: %s') % status['error']
+        task.process_event('fail')
+    elif status.get('finished'):
+        LOG.info(_LI('Inspection finished successfully for node %s'),
+                 node.uuid)
+        task.process_event('done')

ironic/tests/drivers/test_discoverd.py

@@ -0,0 +1,187 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import eventlet
+import ironic_discoverd
+from ironic_discoverd import client
+import mock
+
+from ironic.common import driver_factory
+from ironic.common import exception
+from ironic.common import keystone
+from ironic.common import states
+from ironic.conductor import task_manager
+from ironic.drivers.modules import discoverd
+from ironic.tests.conductor import utils as mgr_utils
+from ironic.tests.db import base as db_base
+from ironic.tests.objects import utils as obj_utils
+
+
+class BaseTestCase(db_base.DbTestCase):
+    def setUp(self):
+        super(BaseTestCase, self).setUp()
+        mgr_utils.mock_the_extension_manager("fake_discoverd")
+        self.driver = driver_factory.get_driver("fake_discoverd")
+        self.node = obj_utils.get_test_node(self.context)
+        self.task = mock.Mock(spec=task_manager.TaskManager)
+        self.task.context = mock.Mock()
+        self.task.shared = False
+        self.task.node = self.node
+        self.task.driver = self.driver
+        self.config(enabled=True, group='discoverd')
+
+
+class ValidateTestCase(BaseTestCase):
+    def test_validate_ok(self):
+        self.driver.inspect.validate(self.task)
+
+    def test_validate_disabled(self):
+        self.config(enabled=False, group='discoverd')
+        self.assertRaises(exception.UnsupportedDriverExtension,
+                          self.driver.inspect.validate, self.task)
+
+    @mock.patch.object(ironic_discoverd, '__version_info__', (1, 0, 0))
+    def test_init_ok(self):
+        discoverd.DiscoverdInspect()
+        self.config(enabled=False, group='discoverd')
+        discoverd.DiscoverdInspect()
+
+    @mock.patch.object(ironic_discoverd, '__version_info__', (0, 2, 2))
+    def test_old_version(self):
+        self.assertRaises(exception.DriverLoadError,
+                          discoverd.DiscoverdInspect)
+        self.config(enabled=False, group='discoverd')
+        discoverd.DiscoverdInspect()
+
+    @mock.patch.object(discoverd, 'ironic_discoverd', None)
+    def test_not_imported(self):
+        self.assertRaises(exception.DriverLoadError,
+                          discoverd.DiscoverdInspect)
+        self.config(enabled=False, group='discoverd')
+        discoverd.DiscoverdInspect()
+
+
+@mock.patch.object(eventlet, 'spawn_n', lambda f, *a, **kw: f(*a, **kw))
+@mock.patch.object(client, 'introspect')
+class InspectHardwareTestCase(BaseTestCase):
+    def test_ok(self, mock_introspect):
+        self.assertEqual(states.INSPECTING,
+                         self.driver.inspect.inspect_hardware(self.task))
+        mock_introspect.assert_called_once_with(
+            self.node.uuid,
+            auth_token=self.task.context.auth_token)
+
+    def test_url(self, mock_introspect):
+        self.config(service_url='meow', group='discoverd')
+        self.assertEqual(states.INSPECTING,
+                         self.driver.inspect.inspect_hardware(self.task))
+        mock_introspect.assert_called_once_with(
+            self.node.uuid,
+            auth_token=self.task.context.auth_token,
+            base_url='meow')
+
+    @mock.patch.object(task_manager, 'acquire')
+    def test_error(self, mock_acquire, mock_introspect):
+        mock_introspect.side_effect = RuntimeError('boom')
+        self.driver.inspect.inspect_hardware(self.task)
+        mock_introspect.assert_called_once_with(
+            self.node.uuid,
+            auth_token=self.task.context.auth_token)
+        task = mock_acquire.return_value.__enter__.return_value
+        self.assertIn('boom', task.node.last_error)
+        task.process_event.assert_called_once_with('fail')
+
+
+@mock.patch.object(keystone, 'get_admin_auth_token', lambda: 'the token')
+@mock.patch.object(client, 'get_status')
+class CheckStatusTestCase(BaseTestCase):
+    def setUp(self):
+        super(CheckStatusTestCase, self).setUp()
+        self.node.provision_state = states.INSPECTING
+
+    def test_not_inspecting(self, mock_get):
+        self.node.provision_state = states.MANAGEABLE
+        discoverd._check_status(self.task)
+        self.assertFalse(mock_get.called)
+
+    def test_not_discoverd(self, mock_get):
+        self.task.driver.inspect = object()
+        discoverd._check_status(self.task)
+        self.assertFalse(mock_get.called)
+
+    def test_not_finished(self, mock_get):
+        mock_get.return_value = {}
+        discoverd._check_status(self.task)
+        mock_get.assert_called_once_with(self.node.uuid,
+                                         auth_token='the token')
+        self.assertFalse(self.task.process_event.called)
+
+    def test_exception_ignored(self, mock_get):
+        mock_get.side_effect = RuntimeError('boom')
+        discoverd._check_status(self.task)
+        mock_get.assert_called_once_with(self.node.uuid,
+                                         auth_token='the token')
+        self.assertFalse(self.task.process_event.called)
+
+    def test_status_ok(self, mock_get):
+        mock_get.return_value = {'finished': True}
+        discoverd._check_status(self.task)
+        mock_get.assert_called_once_with(self.node.uuid,
+                                         auth_token='the token')
+        self.task.process_event.assert_called_once_with('done')
+
+    def test_status_error(self, mock_get):
+        mock_get.return_value = {'error': 'boom'}
+        discoverd._check_status(self.task)
+        mock_get.assert_called_once_with(self.node.uuid,
+                                         auth_token='the token')
+        self.task.process_event.assert_called_once_with('fail')
+        self.assertIn('boom', self.node.last_error)
+
+    def test_service_url(self, mock_get):
+        self.config(service_url='meow', group='discoverd')
+        mock_get.return_value = {'finished': True}
+        discoverd._check_status(self.task)
+        mock_get.assert_called_once_with(self.node.uuid,
+                                         auth_token='the token',
+                                         base_url='meow')
+        self.task.process_event.assert_called_once_with('done')
+
+
+@mock.patch.object(eventlet.greenthread, 'spawn_n',
+                   lambda f, *a, **kw: f(*a, **kw))
+@mock.patch.object(ironic_discoverd, '__version_info__', (1, 0, 0))
+@mock.patch.object(task_manager, 'acquire', autospec=True)
+@mock.patch.object(discoverd, '_check_status', autospec=True)
+class PeriodicTaskTestCase(BaseTestCase):
+    def test_ok(self, mock_check, mock_acquire):
+        mgr = mock.Mock(spec=['iter_nodes'])
+        mgr.iter_nodes.return_value = [('1', 'd1'), ('2', 'd2')]
+        tasks = [mock.sentinel.task1, mock.sentinel.task2]
+        mock_acquire.side_effect = (
+            mock.MagicMock(__enter__=mock.Mock(return_value=task))
+            for task in tasks
+        )
+        discoverd.DiscoverdInspect()._periodic_check_result(
+            mgr, mock.sentinel.context)
+        mock_check.assert_any_call(tasks[0])
+        mock_check.assert_any_call(tasks[1])
+        self.assertEqual(2, mock_acquire.call_count)
+
+    def test_node_locked(self, mock_check, mock_acquire):
+        mgr = mock.Mock(spec=['iter_nodes'])
+        mgr.iter_nodes.return_value = [('1', 'd1'), ('2', 'd2')]
+        mock_acquire.side_effect = exception.NodeLocked("boom")
+        discoverd.DiscoverdInspect()._periodic_check_result(
+            mgr, mock.sentinel.context)
+        self.assertFalse(mock_check.called)
+        self.assertEqual(2, mock_acquire.call_count)

ironic/tests/drivers/third_party_driver_mocks.py

@@ -176,3 +176,14 @@ if not pyremotevbox:
     sys.modules['pyremotevbox'] = pyremotevbox
     if 'ironic.drivers.modules.virtualbox' in sys.modules:
         reload(sys.modules['ironic.drivers.modules.virtualbox'])
+
+
+ironic_discoverd = importutils.try_import('ironic_discoverd')
+if not ironic_discoverd:
+    ironic_discoverd = mock.MagicMock()
+    ironic_discoverd.__version_info__ = (1, 0, 0)
+    ironic_discoverd.__version__ = "1.0.0"
+    sys.modules['ironic_discoverd'] = ironic_discoverd
+    sys.modules['ironic_discoverd.client'] = ironic_discoverd.client
+    if 'ironic.drivers.modules.discoverd' in sys.modules:
+        reload(sys.modules['ironic.drivers.modules.discoverd'])

setup.cfg

@@ -41,6 +41,7 @@ ironic.drivers =
     agent_vbox = ironic.drivers.agent:AgentAndVirtualBoxDriver
     fake = ironic.drivers.fake:FakeDriver
     fake_agent = ironic.drivers.fake:FakeAgentDriver
+    fake_discoverd = ironic.drivers.fake:FakeIPMIToolDiscoverdDriver
     fake_ipmitool = ironic.drivers.fake:FakeIPMIToolDriver
     fake_ipminative = ironic.drivers.fake:FakeIPMINativeDriver
     fake_ssh = ironic.drivers.fake:FakeSSHDriver