openstack/neutron-vpnaas
Code Issues Proposed changes

VPNaaS: Refactoring to use callback mechanism

Browse Source 
This is part 2 of the refactoering that goes along with the changes
in Neutron from review 164466. For this to run, it depends on that
patch.

The change does several things. First, it uses the new callback
mechanism. Note: This mechanism doesn't currently support defining
cass methods as callbacks, so standalone methods are used.

Second, it attempts to remove the need (as much as possible) for
the device drivers to be accepting the VpnService object in __init__().
The idea is to totally remove the argument. However, the Vyatta driver,
as currently implemented, needs it. Hopefully a follow-up can remove
this arg, and instead, will just pass down a config object, so that
tests can override config settings. This also fixes naming of places
that had agent, which are really using vpn_service.

Third, the AdvancedService ABC is removed as a base class for the
VPNService class. Without the L3 agent being saved, and not using
the L3 agent config, the class is not needed.

After this commit, the final step will be to remove the event observer
callback mechanism and AdvancedService class from neutron (once FWaaS
is updated too).

Change-Id: If5040a827a6903cc7cb5e59cdb7fb95f61b13d47
Partial-Bug: #1433552
Depends-On: If134947957fd671aa99a0b2d2b37f7ec65e37766
This commit is contained in:
Paul Michali 2015-03-17 17:09:26 -04:00
parent 77c51c85ce
commit f1a50db9d9
8 changed files with 74 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
neutron_vpnaas/services/vpn/agent.py
View File

@ -34,10 +34,8 @@ class VPNAgent(l3_agent.L3NATAgentWithStateReport):
"""VPNAgent class which can handle vpn service drivers."""
def __init__(self, host, conf=None):
super(VPNAgent, self).__init__(host=host, conf=conf)
# NOTE: Temp location for creating service and loading drivers
self.service = vpn_service.VPNService(self)
self.event_observers.add(self.service)
self.devices = self.service.load_device_drivers(host)
self.device_drivers = self.service.load_device_drivers(host)
def main():

6
neutron_vpnaas/services/vpn/device_drivers/cisco_ipsec.py
View File

@ -100,7 +100,9 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver):
# 1.0 Initial version
target = oslo_messaging.Target(version='1.0')
def __init__(self, agent, host):
def __init__(self, vpn_service, host):
# TODO(pc_m): Once all driver implementations no longer need
# vpn_service argument, replace with just config argument.
self.host = host
self.conn = n_rpc.create_connection(new=True)
context = ctx.get_admin_context_without_session()
@ -116,7 +118,7 @@ class CiscoCsrIPsecDriver(device_drivers.DeviceDriver):
self.periodic_report = loopingcall.FixedIntervalLoopingCall(
self.report_status, context)
self.periodic_report.start(
interval=agent.conf.cisco_csr_ipsec.status_check_interval)
interval=vpn_service.conf.cisco_csr_ipsec.status_check_interval)
LOG.debug("Device driver initialized for %s", node_topic)
def vpnservice_updated(self, context, **kwargs):

8
neutron_vpnaas/services/vpn/device_drivers/ipsec.py
View File

@ -478,7 +478,6 @@ class OpenSwanProcess(BaseSwanProcess):
'--ctlbase', self.pid_path,
'--shutdown',
])
#clean connection_status info
self.connection_status = {}
@ -522,9 +521,10 @@ class IPsecDriver(device_drivers.DeviceDriver):
# 1.0 Initial version
target = oslo_messaging.Target(version='1.0')
def __init__(self, agent, host):
self.agent = agent
self.conf = self.agent.conf
def __init__(self, vpn_service, host):
# TODO(pc_m) Replace vpn_service with config arg, once all driver
# implemenations no longer need vpn_service.
self.conf = vpn_service.conf
self.host = host
self.conn = n_rpc.create_connection(new=True)
self.context = context.get_admin_context_without_session()

65
neutron_vpnaas/services/vpn/vpn_service.py
View File

@ -13,8 +13,9 @@
# License for the specific language governing permissions and limitations
# under the License.
from neutron import context as n_context
from neutron.services import advanced_service
from neutron.callbacks import events
from neutron.callbacks import registry
from neutron.callbacks import resources
from neutron.services import provider_configuration as provconfig
from oslo_config import cfg
from oslo_log import log as logging
@ -27,45 +28,55 @@ LOG = logging.getLogger(__name__)
DEVICE_DRIVERS = 'device_drivers'
class VPNService(advanced_service.AdvancedService):
class VPNService(object):
"""VPN Service observer."""
def __init__(self, l3_agent):
"""Creates a VPN Service instance with context.
"""
self.context = n_context.get_admin_context_without_session()
super(VPNService, self).__init__(l3_agent)
"""Creates a VPN Service instance with context."""
# TODO(pc_m): Replace l3_agent argument with config, once none of the
# device driver implementations need the L3 agent.
self.conf = l3_agent.conf
registry.subscribe(
router_added_actions, resources.ROUTER, events.AFTER_CREATE)
registry.subscribe(
router_removed_actions, resources.ROUTER, events.AFTER_DELETE)
registry.subscribe(
router_updated_actions, resources.ROUTER, events.AFTER_UPDATE)
def load_device_drivers(self, host):
"""Loads one or more device drivers for VPNaaS."""
self.devices = []
drivers = []
for device_driver in cfg.CONF.vpnagent.vpn_device_driver:
device_driver = provconfig.get_provider_driver_class(
device_driver, DEVICE_DRIVERS)
try:
self.devices.append(importutils.import_object(device_driver,
self,
host))
drivers.append(importutils.import_object(device_driver,
self,
host))
LOG.debug('Loaded VPNaaS device driver: %s', device_driver)
except ImportError:
raise vpnaas.DeviceDriverImportError(
device_driver=device_driver)
return self.devices
return drivers
# Overridden handlers for L3 agent events.
def after_router_added(self, ri):
"""Create the router and sync for each loaded device driver."""
for device in self.devices:
device.create_router(ri)
device.sync(self.context, [ri.router])
def after_router_removed(self, ri):
"""Remove the router from each loaded device driver."""
for device in self.devices:
device.destroy_router(ri.router_id)
def router_added_actions(resource, event, l3_agent, **kwargs):
"""Create the router and sync for each loaded device driver."""
router = kwargs['router']
for device_driver in l3_agent.device_drivers:
device_driver.create_router(router)
device_driver.sync(l3_agent.context, [router.router])
def after_router_updated(self, ri):
"""Perform a sync on each loaded device driver."""
for device in self.devices:
device.sync(self.context, [ri.router])
def router_removed_actions(resource, event, l3_agent, **kwargs):
"""Remove the router from each loaded device driver."""
router = kwargs['router']
for device_driver in l3_agent.device_drivers:
device_driver.destroy_router(router.router_id)
def router_updated_actions(resource, event, l3_agent, **kwargs):
"""Perform a sync on each loaded device driver."""
router = kwargs['router']
for device_driver in l3_agent.device_drivers:
device_driver.sync(l3_agent.context, [router.router])

18
neutron_vpnaas/services/vpn/vyatta_agent.py
View File

@ -38,23 +38,7 @@ class VyattaVPNAgent(vyatta_l3.L3AgentMiddleware):
def __init__(self, host, conf=None):
super(VyattaVPNAgent, self).__init__(host, conf)
self.service = vyatta_vpn_service.VyattaVPNService(self)
self.event_observers.add(self.service)
self.devices = self.service.load_device_drivers(host)
def _router_added(self, router_id, router):
super(VyattaVPNAgent, self)._router_added(router_id, router)
for device in self.devices:
device.create_router(router_id)
def _router_removed(self, router_id):
for device in self.devices:
device.destroy_router(router_id)
super(VyattaVPNAgent, self)._router_removed(router_id)
def _process_router_if_compatible(self, router):
super(VyattaVPNAgent, self)._process_router_if_compatible(router)
for device in self.devices:
device.sync(self.context, None)
self.device_drivers = self.service.load_device_drivers(host)
def main():

1
neutron_vpnaas/services/vpn/vyatta_vpn_service.py
View File

@ -28,6 +28,7 @@ class VyattaVPNService(vpn_service.VPNService):
to access router related methods
"""
super(VyattaVPNService, self).__init__(l3_agent)
self.l3_agent = l3_agent
def get_router_client(self, router_id):
"""

2
neutron_vpnaas/tests/functional/strongswan/test_strongswan_driver.py
View File

@ -108,7 +108,7 @@ class TestStrongSwanDeviceDriver(base.BaseSudoTestCase):
looping_call_p.start()
self.driver = strongswan_ipsec.StrongSwanDriver(
agent=mock.Mock(), host=mock.sentinel.host)
vpn_service=mock.Mock(), host=mock.sentinel.host)
self.driver.routers[FAKE_ROUTER_ID] = self.router
self.driver.agent_rpc = mock.Mock()
self.driver._update_nat = mock.Mock()

38
neutron_vpnaas/tests/unit/services/vpn/test_vpn_service.py
View File

@ -15,6 +15,7 @@
import mock
from neutron.agent.l3 import legacy_router
from neutron.callbacks import registry
from neutron.openstack.common import uuidutils
from oslo_config import cfg
@ -50,6 +51,7 @@ class VPNBaseTestCase(base.BaseTestCase):
def setUp(self):
super(VPNBaseTestCase, self).setUp()
self.conf = cfg.CONF
self.conf.use_namespaces = True
self.ri_kwargs = {'router': {'id': FAKE_ROUTER_ID},
'agent_conf': self.conf,
'interface_driver': mock.sentinel.interface_driver}
@ -61,6 +63,7 @@ class TestVirtualPrivateNetworkDeviceDriverLoading(VPNBaseTestCase):
super(TestVirtualPrivateNetworkDeviceDriverLoading, self).setUp()
cfg.CONF.register_opts(vpn_agent.vpn_agent_opts, 'vpnagent')
self.agent = mock.Mock()
mock.patch.object(registry, 'subscribe').start()
self.service = vpn_service.VPNService(self.agent)
def test_loading_vpn_device_drivers(self):
@ -93,24 +96,31 @@ class TestVPNServiceEventHandlers(VPNBaseTestCase):
def setUp(self):
super(TestVPNServiceEventHandlers, self).setUp()
self.l3_agent = mock.Mock()
self.l3_agent.context = mock.sentinel.context
mock.patch.object(registry, 'subscribe').start()
self.service = vpn_service.VPNService(mock.Mock())
self.device = mock.Mock()
self.service.devices = [self.device]
self.device_driver = mock.Mock()
self.l3_agent.device_drivers = [self.device_driver]
def test_actions_after_router_added(self):
def test_router_added_actions(self):
ri = legacy_router.LegacyRouter(FAKE_ROUTER_ID, **self.ri_kwargs)
self.service.after_router_added(ri)
self.device.create_router.assert_called_once_with(ri)
self.device.sync.assert_called_once_with(self.service.context,
[ri.router])
vpn_service.router_added_actions(mock.Mock(), mock.Mock(),
self.l3_agent, router=ri)
self.device_driver.create_router.assert_called_once_with(ri)
self.device_driver.sync.assert_called_once_with(self.l3_agent.context,
[ri.router])
def test_actions_after_router_removed(self):
def test_router_removed_actions(self):
ri = legacy_router.LegacyRouter(FAKE_ROUTER_ID, **self.ri_kwargs)
self.service.after_router_removed(ri)
self.device.destroy_router.assert_called_once_with(FAKE_ROUTER_ID)
vpn_service.router_removed_actions(mock.Mock(), mock.Mock(),
self.l3_agent, router=ri)
self.device_driver.destroy_router.assert_called_once_with(
FAKE_ROUTER_ID)
def test_actions_after_router_updated(self):
def test_router_updated_actions(self):
ri = legacy_router.LegacyRouter(FAKE_ROUTER_ID, **self.ri_kwargs)
self.service.after_router_updated(ri)
self.device.sync.assert_called_once_with(self.service.context,
[ri.router])
vpn_service.router_updated_actions(mock.Mock(), mock.Mock(),
self.l3_agent, router=ri)
self.device_driver.sync.assert_called_once_with(self.l3_agent.context,
[ri.router])