From 29533fe9b6b93ef9c678971828e7cfca09f62cd7 Mon Sep 17 00:00:00 2001 From: Vladyslav Drok Date: Tue, 5 Aug 2014 12:57:10 +0300 Subject: [PATCH] Add driver name on driver load exception When non-DriverLoadError exception occurs during driver loading, it is reraised, providing no information about driver that caused it. In this patch such exceptions are wrapped in DriverLoadError with driver name provided and reraised if driver is enabled. Closes-bug: #1352443 Change-Id: I203618f1c88b940b0122dd1dc204df6325e5e54c --- ironic/common/driver_factory.py | 17 ++++---- ironic/tests/test_driver_factory.py | 61 +++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 ironic/tests/test_driver_factory.py diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index e5f45100c6..446ab2155f 100644 --- a/ironic/common/driver_factory.py +++ b/ironic/common/driver_factory.py @@ -92,18 +92,21 @@ class DriverFactory(object): if cls._extension_manager: return - # NOTE(deva): Drivers raise "DriverNotFound" if they are unable to be + # NOTE(deva): Drivers raise "DriverLoadError" if they are unable to be # loaded, eg. due to missing external dependencies. # We capture that exception, and, only if it is for an - # enabled driver, raise it from here. If the exception - # is for a non-enabled driver, we suppress it. + # enabled driver, raise it from here. If enabled driver + # raises other exception type, it is wrapped in + # "DriverLoadError", providing the name of the driver that + # caused it, and raised. If the exception is for a + # non-enabled driver, we suppress it. def _catch_driver_not_found(mgr, ep, exc): # NOTE(deva): stevedore loads plugins *before* evaluating # _check_func, so we need to check here, too. - if (isinstance(exc, exception.DriverLoadError) and - ep.name not in CONF.enabled_drivers): - return - raise exc + if ep.name in CONF.enabled_drivers: + if not isinstance(exc, exception.DriverLoadError): + raise exception.DriverLoadError(driver=ep.name, reason=exc) + raise exc def _check_func(ext): return ext.name in CONF.enabled_drivers diff --git a/ironic/tests/test_driver_factory.py b/ironic/tests/test_driver_factory.py new file mode 100644 index 0000000000..9749befdf0 --- /dev/null +++ b/ironic/tests/test_driver_factory.py @@ -0,0 +1,61 @@ +# coding=utf-8 + +# 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 mock +from stevedore import dispatch + +from ironic.common import driver_factory +from ironic.common import exception +from ironic.tests import base + + +class FakeEp: + name = 'fake' + + +class DriverLoadTestCase(base.TestCase): + + def setUp(self): + super(DriverLoadTestCase, self).setUp() + driver_factory.DriverFactory._extension_manager = None + + def _fake_init_name_err(self, *args, **kwargs): + kwargs['on_load_failure_callback'](None, FakeEp, NameError('aaa')) + + def _fake_init_driver_err(self, *args, **kwargs): + kwargs['on_load_failure_callback'](None, FakeEp, + exception.DriverLoadError( + driver='aaa', reason='bbb')) + + def test_driver_load_error_if_driver_enabled(self): + self.config(enabled_drivers=['fake']) + with mock.patch.object(dispatch.NameDispatchExtensionManager, + '__init__', self._fake_init_driver_err): + self.assertRaises(exception.DriverLoadError, + driver_factory.DriverFactory._init_extension_manager) + + def test_wrap_in_driver_load_error_if_driver_enabled(self): + self.config(enabled_drivers=['fake']) + with mock.patch.object(dispatch.NameDispatchExtensionManager, + '__init__', self._fake_init_name_err): + self.assertRaises(exception.DriverLoadError, + driver_factory.DriverFactory._init_extension_manager) + + @mock.patch.object(dispatch.NameDispatchExtensionManager, 'names') + def test_no_driver_load_error_if_driver_disabled(self, mock_em): + self.config(enabled_drivers=[]) + with mock.patch.object(dispatch.NameDispatchExtensionManager, + '__init__', self._fake_init_driver_err): + driver_factory.DriverFactory._init_extension_manager() + mock_em.assert_called_once_with()