Merge "Start using in-band inspection"

ironic/common/exception.py

@@ -341,7 +341,8 @@ class OrphanedObjectError(IronicException):
 
 
 class UnsupportedDriverExtension(Invalid):
-    message = _('Driver %(driver)s does not support %(extension)s.')
+    message = _('Driver %(driver)s does not support %(extension)s '
+                '(disabled or not implemented).')
 
 
 class IncompatibleObjectVersion(IronicException):

ironic/drivers/drac.py

@@ -19,6 +19,7 @@ from oslo_utils import importutils
 from ironic.common import exception
 from ironic.common.i18n import _
 from ironic.drivers import base
+from ironic.drivers.modules import discoverd
 from ironic.drivers.modules.drac import management
 from ironic.drivers.modules.drac import power
 from ironic.drivers.modules import pxe
@@ -37,3 +38,5 @@ class PXEDracDriver(base.BaseDriver):
         self.deploy = pxe.PXEDeploy()
         self.management = management.DracManagement()
         self.vendor = pxe.VendorPassthru()
+        self.inspect = discoverd.DiscoverdInspect.create_if_enabled(
+            'PXEDracDriver')

ironic/drivers/fake.py

@@ -215,6 +215,9 @@ class FakeIPMIToolDiscoverdDriver(base.BaseDriver):
         self.deploy = fake.FakeDeploy()
         self.vendor = ipmitool.VendorPassthru()
         self.management = ipmitool.IPMIManagement()
+        # NOTE(dtantsur): unlike other uses of DiscoverdInspect, this one is
+        # unconditional, as this driver is designed for testing discoverd
+        # integration.
         self.inspect = discoverd.DiscoverdInspect()
 
 

ironic/drivers/modules/discoverd.py

@@ -23,6 +23,7 @@ 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.i18n import _LW
 from ironic.common import keystone
 from ironic.common import states
 from ironic.conductor import task_manager
@@ -57,10 +58,25 @@ if ironic_discoverd:
 class DiscoverdInspect(base.InspectInterface):
     """In-band inspection via ironic-discoverd project."""
 
+    @classmethod
+    def create_if_enabled(cls, driver_name):
+        """Create instance of DiscoverdInspect if it's enabled.
+
+        Reports log warning with given driver_name if it's not.
+
+        :return: DiscoverdInspect instance or None
+        """
+        if CONF.discoverd.enabled:
+            return cls()
+        else:
+            LOG.warn(_LW("Inspection via ironic-discoverd is disabled in "
+                         "configuration for driver %s, set "
+                         "[discoverd]enabled = true to enable"), driver_name)
+
     def __init__(self):
         if not CONF.discoverd.enabled:
-            LOG.info('ironic-discoverd support is disabled')
+            raise exception.DriverLoadError(
-            return
+                _('ironic-discoverd support is disabled'))
 
         if not ironic_discoverd:
             raise exception.DriverLoadError(
@@ -70,7 +86,7 @@ class DiscoverdInspect(base.InspectInterface):
         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 '
+                _('ironic-discoverd version is too old: required >= 1.0.0, '
                   'got %s') % '.'.join(str(x) for x in version))
 
     def get_properties(self):
@@ -81,18 +97,15 @@ class DiscoverdInspect(base.InspectInterface):
         return {}  # no properties
 
     def validate(self, task):
-        """Validate the driver-specific management information.
+        """Validate the driver-specific inspection 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:
+        # NOTE(deva): this is not callable if discoverd is disabled
-            raise exception.UnsupportedDriverExtension(
+        #             so don't raise an exception -- just pass.
-                _('ironic-discoverd support is disabled in '
+        pass
-                  'configuration, set [discoverd]enabled to true '
-                  'to enable'))
 
     def inspect_hardware(self, task):
         """Inspect hardware to obtain the hardware properties.
@@ -185,7 +198,8 @@ def _check_status(task):
         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']
+        node.last_error = (_('ironic-discoverd inspection failed: %s')
+                           % status['error'])
         task.process_event('fail')
     elif status.get('finished'):
         LOG.info(_LI('Inspection finished successfully for node %s'),

ironic/drivers/pxe.py

@@ -25,6 +25,7 @@ from ironic.drivers import base
 from ironic.drivers.modules.amt import management as amt_management
 from ironic.drivers.modules.amt import power as amt_power
 from ironic.drivers.modules.amt import vendor as amt_vendor
+from ironic.drivers.modules import discoverd
 from ironic.drivers.modules import iboot
 from ironic.drivers.modules.ilo import deploy as ilo_deploy
 from ironic.drivers.modules.ilo import management as ilo_management
@@ -56,6 +57,8 @@ class PXEAndIPMIToolDriver(base.BaseDriver):
         self.deploy = pxe.PXEDeploy()
         self.management = ipmitool.IPMIManagement()
         self.vendor = pxe.VendorPassthru()
+        self.inspect = discoverd.DiscoverdInspect.create_if_enabled(
+            'PXEAndIPMIToolDriver')
 
 
 class PXEAndSSHDriver(base.BaseDriver):
@@ -75,6 +78,8 @@ class PXEAndSSHDriver(base.BaseDriver):
         self.deploy = pxe.PXEDeploy()
         self.management = ssh.SSHManagement()
         self.vendor = pxe.VendorPassthru()
+        self.inspect = discoverd.DiscoverdInspect.create_if_enabled(
+            'PXEAndSSHDriver')
 
 
 class PXEAndIPMINativeDriver(base.BaseDriver):
@@ -98,6 +103,8 @@ class PXEAndIPMINativeDriver(base.BaseDriver):
         self.deploy = pxe.PXEDeploy()
         self.management = ipminative.NativeIPMIManagement()
         self.vendor = pxe.VendorPassthru()
+        self.inspect = discoverd.DiscoverdInspect.create_if_enabled(
+            'PXEAndIPMINativeDriver')
 
 
 class PXEAndSeaMicroDriver(base.BaseDriver):

ironic/tests/drivers/test_discoverd.py

@@ -26,9 +26,51 @@ from ironic.tests.db import base as db_base
 from ironic.tests.objects import utils as obj_utils
 
 
+class DisabledTestCase(db_base.DbTestCase):
+    def setUp(self):
+        super(DisabledTestCase, self).setUp()
+
+    def _do_mock(self):
+        # NOTE(dtantsur): fake driver always has inspection, using another one
+        mgr_utils.mock_the_extension_manager("pxe_ssh")
+        self.driver = driver_factory.get_driver("pxe_ssh")
+
+    def test_disabled(self):
+        self.config(enabled=False, group='discoverd')
+        self._do_mock()
+        self.assertIsNone(self.driver.inspect)
+        # NOTE(dtantsur): it's expected that fake_discoverd fails to load
+        # in this case
+        self.assertRaises(exception.DriverLoadError,
+                          mgr_utils.mock_the_extension_manager,
+                          "fake_discoverd")
+
+    def test_enabled(self):
+        self.config(enabled=True, group='discoverd')
+        self._do_mock()
+        self.assertIsNotNone(self.driver.inspect)
+
+    @mock.patch.object(discoverd, 'ironic_discoverd', None)
+    def test_init_discoverd_not_imported(self):
+        self.assertRaises(exception.DriverLoadError,
+                          discoverd.DiscoverdInspect)
+
+    @mock.patch.object(ironic_discoverd, '__version_info__', (1, 0, 0))
+    def test_init_ok(self):
+        self.config(enabled=True, group='discoverd')
+        discoverd.DiscoverdInspect()
+
+    @mock.patch.object(ironic_discoverd, '__version_info__', (0, 2, 2))
+    def test_init_old_version(self):
+        self.config(enabled=True, group='discoverd')
+        self.assertRaises(exception.DriverLoadError,
+                          discoverd.DiscoverdInspect)
+
+
 class BaseTestCase(db_base.DbTestCase):
     def setUp(self):
         super(BaseTestCase, self).setUp()
+        self.config(enabled=True, group='discoverd')
         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)
@@ -37,37 +79,26 @@ class BaseTestCase(db_base.DbTestCase):
         self.task.shared = False
         self.task.node = self.node
         self.task.driver = self.driver
-        self.config(enabled=True, group='discoverd')
 
 
-class ValidateTestCase(BaseTestCase):
+class CommonFunctionsTestCase(BaseTestCase):
     def test_validate_ok(self):
         self.driver.inspect.validate(self.task)
 
-    def test_validate_disabled(self):
+    def test_get_properties(self):
-        self.config(enabled=False, group='discoverd')
+        res = self.driver.inspect.get_properties()
-        self.assertRaises(exception.UnsupportedDriverExtension,
+        self.assertEqual({}, res)
-                          self.driver.inspect.validate, self.task)
 
-    @mock.patch.object(ironic_discoverd, '__version_info__', (1, 0, 0))
+    def test_create_if_enabled(self):
-    def test_init_ok(self):
+        res = discoverd.DiscoverdInspect.create_if_enabled('driver')
-        discoverd.DiscoverdInspect()
+        self.assertIsInstance(res, discoverd.DiscoverdInspect)
-        self.config(enabled=False, group='discoverd')
-        discoverd.DiscoverdInspect()
 
-    @mock.patch.object(ironic_discoverd, '__version_info__', (0, 2, 2))
+    @mock.patch.object(discoverd.LOG, 'warn')
-    def test_old_version(self):
+    def test_create_if_enabled_disabled(self, warn_mock):
-        self.assertRaises(exception.DriverLoadError,
-                          discoverd.DiscoverdInspect)
         self.config(enabled=False, group='discoverd')
-        discoverd.DiscoverdInspect()
+        res = discoverd.DiscoverdInspect.create_if_enabled('driver')
-
+        self.assertIsNone(res)
-    @mock.patch.object(discoverd, 'ironic_discoverd', None)
+        self.assertTrue(warn_mock.called)
-    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))