Tighten exception handler for import_object

oslo_utils raise ImportError if import fails. We should propagate other
failures to callers. Otherwise we may hide issues.

Also report exact failure from import_object in case L3 agent fails to
import interface_driver.

As part of the job, consolidated code to load interface driver into
common function.

Also, stopped checking for specific log messages in dhcp and l3 agent
unit tests: it's too fragile and actually not something we need a unit
test for.

Not to introduce more work for people who handle py3 porting effort,
added the unit test into the list of those that are executed for py34
job until the whole suite is ready for python3.

Change-Id: I10cdb8414c9fb4ad5cfd3f3b2630811f50ffb0c7

diff --git a/neutron/agent/common/utils.py b/neutron/agent/common/utils.py
index 2eddabd6db7c45035f21f1ea664b0dce6d068209..2b50da21704d101182b8f2bf8919cb954c6763a8 100644 (file)
--- a/neutron/agent/common/utils.py
+++ b/neutron/agent/common/utils.py
@@ -15,10 +15,33 @@
 
 import os
 
+from oslo_log import log as logging
+from oslo_utils import importutils
+
+from neutron.i18n import _LE
+
 
 if os.name == 'nt':
     from neutron.agent.windows import utils
 else:
     from neutron.agent.linux import utils
 
+
+LOG = logging.getLogger(__name__)
+
+
 execute = utils.execute
+
+
+def load_interface_driver(conf):
+    if not conf.interface_driver:
+        LOG.error(_LE('An interface driver must be specified'))
+        raise SystemExit(1)
+    try:
+        return importutils.import_object(conf.interface_driver, conf)
+    except ImportError as e:
+        LOG.error(_LE("Error importing interface driver "
+                      "'%(driver)s': %(inner)s"),
+                  {'driver': conf.interface_driver,
+                   'inner': e})
+        raise SystemExit(1)

diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py
index e3379bfae65bc6f8143eff5f4e434de92a51e83d..23906bc3d73b6c4a10808bad142a5412a9737934 100644 (file)
--- a/neutron/agent/l3/agent.py
+++ b/neutron/agent/l3/agent.py
@@ -21,9 +21,9 @@ import oslo_messaging
 from oslo_service import loopingcall
 from oslo_service import periodic_task
 from oslo_utils import excutils
-from oslo_utils import importutils
 from oslo_utils import timeutils
 
+from neutron.agent.common import utils as common_utils
 from neutron.agent.l3 import dvr
 from neutron.agent.l3 import dvr_edge_router as dvr_router
 from neutron.agent.l3 import dvr_local_router as dvr_local_router
@@ -165,15 +165,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
             config=self.conf,
             resource_type='router')
 
-        try:
-            self.driver = importutils.import_object(
-                self.conf.interface_driver,
-                self.conf
-            )
-        except Exception:
-            LOG.error(_LE("Error importing interface driver "
-                          "'%s'"), self.conf.interface_driver)
-            raise SystemExit(1)
+        self.driver = common_utils.load_interface_driver(self.conf)
 
         self.context = n_context.get_admin_context_without_session()
         self.plugin_rpc = L3PluginApi(topics.L3PLUGIN, host)

diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py
index d5d748dec4f655b3e51fe8b2cd96450601ba6248..0a06259c1a0d948b4fbab130a00c560dd5e16a1e 100644 (file)
--- a/neutron/agent/linux/dhcp.py
+++ b/neutron/agent/linux/dhcp.py
@@ -23,10 +23,10 @@ import time
 import netaddr
 from oslo_config import cfg
 from oslo_log import log as logging
-from oslo_utils import importutils
 from oslo_utils import uuidutils
 import six
 
+from neutron.agent.common import utils as common_utils
 from neutron.agent.linux import external_process
 from neutron.agent.linux import ip_lib
 from neutron.agent.linux import iptables_manager
@@ -36,7 +36,7 @@ from neutron.common import exceptions
 from neutron.common import ipv6_utils
 from neutron.common import utils as commonutils
 from neutron.extensions import extra_dhcp_opt as edo_ext
-from neutron.i18n import _LE, _LI, _LW
+from neutron.i18n import _LI, _LW
 
 LOG = logging.getLogger(__name__)
 
@@ -919,18 +919,7 @@ class DeviceManager(object):
     def __init__(self, conf, plugin):
         self.conf = conf
         self.plugin = plugin
-        if not conf.interface_driver:
-            LOG.error(_LE('An interface driver must be specified'))
-            raise SystemExit(1)
-        try:
-            self.driver = importutils.import_object(
-                conf.interface_driver, conf)
-        except Exception as e:
-            LOG.error(_LE("Error importing interface driver '%(driver)s': "
-                          "%(inner)s"),
-                      {'driver': conf.interface_driver,
-                       'inner': e})
-            raise SystemExit(1)
+        self.driver = common_utils.load_interface_driver(conf)
 
     def get_interface_name(self, network, port):
         """Return interface(device) name for use by the DHCP process."""

diff --git a/neutron/tests/unit/agent/common/test_utils.py b/neutron/tests/unit/agent/common/test_utils.py
new file mode 100644 (file)
index 0000000..7c89b1e
--- /dev/null
+++ b/neutron/tests/unit/agent/common/test_utils.py
@@ -0,0 +1,53 @@
+# Copyright 2015 Red Hat, Inc.
+# All Rights Reserved.
+#
+#    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 neutron.agent.common import config
+from neutron.agent.common import utils
+from neutron.agent.linux import interface
+from neutron.tests import base
+from neutron.tests.unit import testlib_api
+
+
+class TestLoadInterfaceDriver(base.BaseTestCase):
+
+    def setUp(self):
+        super(TestLoadInterfaceDriver, self).setUp()
+        self.conf = config.setup_conf()
+        config.register_interface_driver_opts_helper(self.conf)
+
+    def test_load_interface_driver_not_set(self):
+        with testlib_api.ExpectedException(SystemExit):
+            utils.load_interface_driver(self.conf)
+
+    def test_load_interface_driver_wrong_driver(self):
+        self.conf.set_override('interface_driver', 'neutron.NonExistentDriver')
+        with testlib_api.ExpectedException(SystemExit):
+            utils.load_interface_driver(self.conf)
+
+    def test_load_interface_driver_does_not_consume_irrelevant_errors(self):
+        self.conf.set_override('interface_driver',
+                               'neutron.agent.linux.interface.NullDriver')
+        with mock.patch('oslo_utils.importutils.import_object',
+                        side_effect=RuntimeError()):
+            with testlib_api.ExpectedException(RuntimeError):
+                utils.load_interface_driver(self.conf)
+
+    def test_load_interface_driver_success(self):
+        self.conf.set_override('interface_driver',
+                               'neutron.agent.linux.interface.NullDriver')
+        self.assertIsInstance(utils.load_interface_driver(self.conf),
+                              interface.NullDriver)

diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py
index 876bf8db424ea75decbaf971a4a9a66ccc324992..96ddd988e1be36bad0455a93b7e3c3249b3c87bf 100644 (file)
--- a/neutron/tests/unit/agent/dhcp/test_agent.py
+++ b/neutron/tests/unit/agent/dhcp/test_agent.py
@@ -438,22 +438,17 @@ class TestDhcpAgent(base.BaseTestCase):
 
     def test_none_interface_driver(self):
         cfg.CONF.set_override('interface_driver', None)
-        with mock.patch.object(dhcp, 'LOG') as log:
-            self.assertRaises(SystemExit, dhcp.DeviceManager,
-                              cfg.CONF, None)
-            msg = 'An interface driver must be specified'
-            log.error.assert_called_once_with(msg)
+        self.assertRaises(SystemExit, dhcp.DeviceManager,
+                          cfg.CONF, None)
 
     def test_nonexistent_interface_driver(self):
         # Temporarily turn off mock, so could use the real import_class
         # to import interface_driver.
         self.driver_cls_p.stop()
         self.addCleanup(self.driver_cls_p.start)
-        cfg.CONF.set_override('interface_driver', 'foo')
-        with mock.patch.object(dhcp, 'LOG') as log:
-            self.assertRaises(SystemExit, dhcp.DeviceManager,
-                              cfg.CONF, None)
-            self.assertEqual(log.error.call_count, 1)
+        cfg.CONF.set_override('interface_driver', 'foo.bar')
+        self.assertRaises(SystemExit, dhcp.DeviceManager,
+                          cfg.CONF, None)
 
 
 class TestLogArgs(base.BaseTestCase):

diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py
index b683727fdb59d2ff41a0f2985d74cea9995cb0c3..6c3b74380746e9d91f9d653626213e302454dbc2 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_agent.py
+++ b/neutron/tests/unit/agent/l3/test_agent.py
@@ -44,7 +44,6 @@ from neutron.agent import rpc as agent_rpc
 from neutron.common import config as base_config
 from neutron.common import constants as l3_constants
 from neutron.common import exceptions as n_exc
-from neutron.i18n import _LE
 from neutron.plugins.common import constants as p_const
 from neutron.tests import base
 from neutron.tests.common import l3_test_common
@@ -1852,18 +1851,12 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
 
     def test_nonexistent_interface_driver(self):
         self.conf.set_override('interface_driver', None)
-        with mock.patch.object(l3_agent, 'LOG') as log:
-            self.assertRaises(SystemExit, l3_agent.L3NATAgent,
-                              HOSTNAME, self.conf)
-            msg = 'An interface driver must be specified'
-            log.error.assert_called_once_with(msg)
-
-        self.conf.set_override('interface_driver', 'wrong_driver')
-        with mock.patch.object(l3_agent, 'LOG') as log:
-            self.assertRaises(SystemExit, l3_agent.L3NATAgent,
-                              HOSTNAME, self.conf)
-            msg = _LE("Error importing interface driver '%s'")
-            log.error.assert_called_once_with(msg, 'wrong_driver')
+        self.assertRaises(SystemExit, l3_agent.L3NATAgent,
+                          HOSTNAME, self.conf)
+
+        self.conf.set_override('interface_driver', 'wrong.driver')
+        self.assertRaises(SystemExit, l3_agent.L3NATAgent,
+                          HOSTNAME, self.conf)
 
     @mock.patch.object(namespaces.RouterNamespace, 'delete')
     @mock.patch.object(dvr_snat_ns.SnatNamespace, 'delete')

diff --git a/tox.ini b/tox.ini
index f473cd350a8e55bbddcacab9880c296f3131bb39..0f2431e30df230c9596350822061bfbe5b6863c8 100644 (file)
--- a/tox.ini
+++ b/tox.ini
@@ -177,6 +177,7 @@ commands = python -m testtools.run \
     neutron.tests.unit.agent.l3.test_dvr_fip_ns \
     neutron.tests.unit.agent.common.test_config \
     neutron.tests.unit.agent.common.test_polling \
+    neutron.tests.unit.agent.common.test_utils \
     neutron.tests.unit.agent.linux.test_ip_lib \
     neutron.tests.unit.agent.linux.test_keepalived \
     neutron.tests.unit.agent.linux.test_daemon \