openstack
/
neutron
Code Issues Proposed changes

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
This commit is contained in:
Ihar Hrachyshka 2015-07-13 11:49:42 +02:00
parent 1ef2fc8fa1
commit 5d07ec908c
7 changed files with 92 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
neutron/agent/common/utils.py
View File

@ -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)

12
neutron/agent/l3/agent.py
View File

@ -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)

17
neutron/agent/linux/dhcp.py
View File

@ -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."""

53
neutron/tests/unit/agent/common/test_utils.py Normal file
View File

@ -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)

15
neutron/tests/unit/agent/dhcp/test_agent.py
View File

@ -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):

17
neutron/tests/unit/agent/l3/test_agent.py
View File

@ -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.assertRaises(SystemExit, l3_agent.L3NATAgent,
HOSTNAME, self.conf)
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.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')

1
tox.ini
View File

@ -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 \