diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index a11dc3f68c..f6b62c9234 100644 --- a/ironic/common/driver_factory.py +++ b/ironic/common/driver_factory.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import collections + from oslo_concurrency import lockutils from oslo_config import cfg from oslo_log import log @@ -69,7 +71,12 @@ def get_driver(driver_name): def drivers(): """Get all drivers as a dict name -> driver object.""" factory = DriverFactory() - return {name: factory[name].obj for name in factory.names} + # NOTE(jroll) I don't think this needs to be ordered, but + # ConductorManager.init_host seems to depend on this behavior (or at + # least the unit tests for it do), and it can't hurt much to keep it + # that way. + return collections.OrderedDict((name, factory[name].obj) + for name in factory.names) class DriverFactory(object): diff --git a/ironic/conductor/base_manager.py b/ironic/conductor/base_manager.py index e4ace43cca..da5d9daf22 100644 --- a/ironic/conductor/base_manager.py +++ b/ironic/conductor/base_manager.py @@ -66,26 +66,14 @@ class BaseConductorManager(object): self.notifier = rpc.get_notifier() self._started = False - def _get_driver(self, driver_name): - """Get the driver. - - :param driver_name: name of the driver. - :returns: the driver; an instance of a class which implements - :class:`ironic.drivers.base.BaseDriver`. - :raises: DriverNotFound if the driver is not loaded. - - """ - try: - return self._driver_factory[driver_name].obj - except KeyError: - raise exception.DriverNotFound(driver_name=driver_name) - def init_host(self, admin_context=None): """Initialize the conductor host. :param admin_context: the admin context to pass to periodic tasks. - :raises: RuntimeError when conductor is already running - :raises: NoDriversLoaded when no drivers are enabled on the conductor + :raises: RuntimeError when conductor is already running. + :raises: NoDriversLoaded when no drivers are enabled on the conductor. + :raises: DriverNotFound if a driver is enabled that does not exist. + :raises: DriverLoadError if an enabled driver cannot be loaded. """ if self._started: raise RuntimeError(_('Attempt to start an already running ' @@ -107,21 +95,19 @@ class BaseConductorManager(object): self.ring_manager = hash.HashRingManager() """Consistent hash ring which maps drivers to conductors.""" - # NOTE(deva): instantiating DriverFactory may raise DriverLoadError - # or DriverNotFound - self._driver_factory = driver_factory.DriverFactory() - """Driver factory loads all enabled drivers.""" - - self.drivers = self._driver_factory.names - """List of driver names which this conductor supports.""" - - if not self.drivers: + # NOTE(deva): this call may raise DriverLoadError or DriverNotFound + drivers = driver_factory.drivers() + if not drivers: msg = _LE("Conductor %s cannot be started because no drivers " "were loaded. This could be because no drivers were " "specified in 'enabled_drivers' config option.") LOG.error(msg, self.host) raise exception.NoDriversLoaded(conductor=self.host) + # NOTE(jroll) this is passed to the dbapi, which requires a list, not + # a generator (which keys() returns in py3) + driver_names = list(drivers) + # Collect driver-specific periodic tasks. # Conductor periodic tasks accept context argument, driver periodic # tasks accept this manager and context. We have to ensure that the @@ -131,7 +117,7 @@ class BaseConductorManager(object): self._periodic_task_callables = [] periodic_task_classes = set() self._collect_periodic_tasks(self, (admin_context,)) - for driver_obj in driver_factory.drivers().values(): + for driver_obj in drivers.values(): self._collect_periodic_tasks(driver_obj, (self, admin_context)) for iface_name in (driver_obj.core_interfaces + driver_obj.standard_interfaces + @@ -158,7 +144,7 @@ class BaseConductorManager(object): try: # Register this conductor with the cluster cdr = self.dbapi.register_conductor({'hostname': self.host, - 'drivers': self.drivers}) + 'drivers': driver_names}) except exception.ConductorAlreadyRegistered: # This conductor was already registered and did not shut down # properly, so log a warning and update the record. @@ -167,7 +153,7 @@ class BaseConductorManager(object): "was previously registered. Updating registration"), {'hostname': self.host}) cdr = self.dbapi.register_conductor({'hostname': self.host, - 'drivers': self.drivers}, + 'drivers': driver_names}, update_existing=True) self.conductor = cdr diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index fe4c49d67f..b4bde13982 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -54,6 +54,7 @@ from oslo_utils import excutils from oslo_utils import uuidutils from ironic.common import dhcp_factory +from ironic.common import driver_factory from ironic.common import exception from ironic.common.glance_service import service_utils as glance_utils from ironic.common.i18n import _ @@ -394,7 +395,7 @@ class ConductorManager(base_manager.BaseConductorManager): # Any locking in a top-level vendor action will need to be done by the # implementation, as there is little we could reasonably lock on here. LOG.debug("RPC driver_vendor_passthru for driver %s." % driver_name) - driver = self._get_driver(driver_name) + driver = driver_factory.get_driver(driver_name) if not getattr(driver, 'vendor', None): raise exception.UnsupportedDriverExtension( driver=driver_name, @@ -466,7 +467,7 @@ class ConductorManager(base_manager.BaseConductorManager): # implementation, as there is little we could reasonably lock on here. LOG.debug("RPC get_driver_vendor_passthru_methods for driver %s" % driver_name) - driver = self._get_driver(driver_name) + driver = driver_factory.get_driver(driver_name) if not getattr(driver, 'vendor', None): raise exception.UnsupportedDriverExtension( driver=driver_name, @@ -1818,7 +1819,7 @@ class ConductorManager(base_manager.BaseConductorManager): """ LOG.debug("RPC get_driver_properties called for driver %s.", driver_name) - driver = self._get_driver(driver_name) + driver = driver_factory.get_driver(driver_name) return driver.get_properties() @periodics.periodic(spacing=CONF.conductor.send_sensor_data_interval) @@ -2130,7 +2131,7 @@ class ConductorManager(base_manager.BaseConductorManager): LOG.debug("RPC get_raid_logical_disk_properties " "called for driver %s" % driver_name) - driver = self._get_driver(driver_name) + driver = driver_factory.get_driver(driver_name) if not getattr(driver, 'raid', None): raise exception.UnsupportedDriverExtension( driver=driver_name, extension='raid') diff --git a/ironic/tests/unit/common/test_driver_factory.py b/ironic/tests/unit/common/test_driver_factory.py index 8cc49e46dd..fae28f46a9 100644 --- a/ironic/tests/unit/common/test_driver_factory.py +++ b/ironic/tests/unit/common/test_driver_factory.py @@ -17,6 +17,7 @@ from stevedore import dispatch from ironic.common import driver_factory from ironic.common import exception +from ironic.drivers import base as drivers_base from ironic.tests import base @@ -62,3 +63,18 @@ class DriverLoadTestCase(base.TestCase): '__init__', self._fake_init_driver_err): driver_factory.DriverFactory._init_extension_manager() self.assertEqual(2, mock_em.call_count) + + +class GetDriverTestCase(base.TestCase): + def setUp(self): + super(GetDriverTestCase, self).setUp() + driver_factory.DriverFactory._extension_manager = None + self.config(enabled_drivers=['fake']) + + def test_get_driver_known(self): + driver = driver_factory.get_driver('fake') + self.assertIsInstance(driver, drivers_base.BaseDriver) + + def test_get_driver_unknown(self): + self.assertRaises(exception.DriverNotFound, + driver_factory.get_driver, 'unknown_driver') diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 4d0c3e8290..98ba339660 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2299,23 +2299,12 @@ class DoNodeVerifyTestCase(mgr_utils.ServiceSetUpMixin, @mgr_utils.mock_record_keepalive class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn, tests_db_base.DbTestCase): - def test_get_driver_known(self): - self._start_service() - driver = self.service._get_driver('fake') - self.assertIsInstance(driver, drivers_base.BaseDriver) - - def test_get_driver_unknown(self): - self._start_service() - self.assertRaises(exception.DriverNotFound, - self.service._get_driver, 'unknown_driver') - def test__mapped_to_this_conductor(self): self._start_service() n = utils.get_test_node() self.assertTrue(self.service._mapped_to_this_conductor(n['uuid'], 'fake')) self.assertFalse(self.service._mapped_to_this_conductor(n['uuid'], - 'otherdriver')) @mock.patch.object(images, 'is_whole_disk_image')