Refactoring cleanup for L3 agent callbacks
This commit completes the refactoring of the L3 agent callback mechanism. The goal here is to also use the neutron/callbacks/ mechanism for L3 agent notifications, instead of have two mechanisms. [1] modified the L3 agent to send notifiactions for router create, udpate, and delete events, using the neutron/callbacks/ mechanism. [2] modified VPN to use this new mechanism, instead of the L3EventObservers mechanism. Note: [3] modified FW repo to no longer depended on the L3EventObserver and related objects (it doesn't currently use the event notifications). This commit removes the notifications for the L3EventObservers mechanism, removed the related modules and tests, and adds in tests to verify that the new notifications are called for the different events. Once [1] and [2] are upstreamed, this commit can proceed. Refs: [1] https://review.openstack.org/#/c/164466/ [2] https://review.openstack.org/#/c/165226/ [3] https://review.openstack.org/#/c/167275/ Change-Id: I7c4b4ea5f9fb19abb812665cdae5fb70c84fe3ec Depends-On: If5040a827a6903cc7cb5e59cdb7fb95f61b13d47 Closes-Bug: #1433552
This commit is contained in:
Paul Michali
parent
30c2e203d9
commit
9bca9ca84b
|
|
@ -24,7 +24,6 @@ from oslo_utils import timeutils
|
|||
|
||||
from neutron.agent.l3 import dvr
|
||||
from neutron.agent.l3 import dvr_router
|
||||
from neutron.agent.l3 import event_observers
|
||||
from neutron.agent.l3 import ha
|
||||
from neutron.agent.l3 import ha_router
|
||||
from neutron.agent.l3 import legacy_router
|
||||
|
|
@ -48,7 +47,7 @@ from neutron.i18n import _LE, _LI, _LW
|
|||
from neutron import manager
|
||||
from neutron.openstack.common import loopingcall
|
||||
from neutron.openstack.common import periodic_task
|
||||
from neutron.services import advanced_service as adv_svc
|
||||
|
||||
try:
|
||||
from neutron_fwaas.services.firewall.agents.l3reference \
|
||||
import firewall_l3_agent
|
||||
|
|
@ -215,7 +214,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
|
|||
self.conf.use_namespaces)
|
||||
|
||||
self._queue = queue.RouterProcessingQueue()
|
||||
self.event_observers = event_observers.L3EventObservers()
|
||||
super(L3NATAgent, self).__init__(conf=self.conf)
|
||||
|
||||
self.target_ex_net_id = None
|
||||
|
|
@ -309,8 +307,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
|
|||
|
||||
def _router_added(self, router_id, router):
|
||||
ri = self._create_router(router_id, router)
|
||||
self.event_observers.notify(
|
||||
adv_svc.AdvancedService.before_router_added, ri)
|
||||
registry.notify(resources.ROUTER, events.BEFORE_CREATE,
|
||||
self, router=ri)
|
||||
|
||||
|
|
@ -328,16 +324,12 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
|
|||
"Skipping router removal"), router_id)
|
||||
return
|
||||
|
||||
self.event_observers.notify(
|
||||
adv_svc.AdvancedService.before_router_removed, ri)
|
||||
registry.notify(resources.ROUTER, events.BEFORE_DELETE,
|
||||
self, router=ri)
|
||||
|
||||
ri.delete(self)
|
||||
del self.router_info[router_id]
|
||||
|
||||
self.event_observers.notify(
|
||||
adv_svc.AdvancedService.after_router_removed, ri)
|
||||
registry.notify(resources.ROUTER, events.AFTER_DELETE, self, router=ri)
|
||||
|
||||
def update_fip_statuses(self, ri, existing_floating_ips, fip_statuses):
|
||||
|
|
@ -418,20 +410,14 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
|
|||
ri = self.router_info[router['id']]
|
||||
ri.router = router
|
||||
ri.process(self)
|
||||
self.event_observers.notify(
|
||||
adv_svc.AdvancedService.after_router_added, ri)
|
||||
registry.notify(resources.ROUTER, events.AFTER_CREATE, self, router=ri)
|
||||
|
||||
def _process_updated_router(self, router):
|
||||
ri = self.router_info[router['id']]
|
||||
ri.router = router
|
||||
self.event_observers.notify(
|
||||
adv_svc.AdvancedService.before_router_updated, ri)
|
||||
registry.notify(resources.ROUTER, events.BEFORE_UPDATE,
|
||||
self, router=ri)
|
||||
ri.process(self)
|
||||
self.event_observers.notify(
|
||||
adv_svc.AdvancedService.after_router_updated, ri)
|
||||
registry.notify(resources.ROUTER, events.AFTER_UPDATE, self, router=ri)
|
||||
|
||||
def _process_router_update(self):
|
||||
|
|
|
|||
|
|
@ -1,40 +0,0 @@
|
|||
# Copyright 2014 OpenStack Foundation
|
||||
# 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.
|
||||
|
||||
|
||||
class L3EventObservers(object):
|
||||
|
||||
"""Manages observers for L3 agent events."""
|
||||
|
||||
def __init__(self):
|
||||
self.observers = set()
|
||||
|
||||
def add(self, new_observer):
|
||||
"""Add a listener for L3 agent notifications."""
|
||||
for observer in self.observers:
|
||||
if isinstance(new_observer, type(observer)):
|
||||
raise ValueError('Only a single instance of AdvancedService '
|
||||
'may be registered, per type of service.')
|
||||
|
||||
self.observers.add(new_observer)
|
||||
|
||||
def notify(self, l3_event_action, *args, **kwargs):
|
||||
"""Give interested parties a chance to act on event.
|
||||
|
||||
NOTE: Preserves existing behavior for error propagation.
|
||||
"""
|
||||
method_name = l3_event_action.__name__
|
||||
for observer in self.observers:
|
||||
getattr(observer, method_name)(*args, **kwargs)
|
||||
|
|
@ -26,7 +26,6 @@ from neutron.callbacks import events
|
|||
from neutron.callbacks import registry
|
||||
from neutron.callbacks import resources
|
||||
from neutron.common import exceptions
|
||||
from neutron.services import advanced_service
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -35,7 +34,7 @@ METADATA_ACCESS_MARK_MASK = '0xffffffff'
|
|||
METADATA_SERVICE_NAME = 'metadata-proxy'
|
||||
|
||||
|
||||
class MetadataDriver(advanced_service.AdvancedService):
|
||||
class MetadataDriver(object):
|
||||
|
||||
OPTS = [
|
||||
cfg.StrOpt('metadata_proxy_socket',
|
||||
|
|
@ -66,7 +65,6 @@ class MetadataDriver(advanced_service.AdvancedService):
|
|||
]
|
||||
|
||||
def __init__(self, l3_agent):
|
||||
super(MetadataDriver, self).__init__(l3_agent)
|
||||
self.metadata_port = l3_agent.conf.metadata_port
|
||||
self.metadata_access_mark = l3_agent.conf.metadata_access_mark
|
||||
registry.subscribe(
|
||||
|
|
|
|||
|
|
@ -1,72 +0,0 @@
|
|||
# Copyright 2014 OpenStack Foundation.
|
||||
# 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.
|
||||
|
||||
from oslo_log import log as logging
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class AdvancedService(object):
|
||||
"""Observer base class for Advanced Services.
|
||||
|
||||
Base class for service types. This should not be instantiated normally.
|
||||
Instead, a child class is defined for each service type and instantiated
|
||||
by the corresponding service agent. The instances will have a back
|
||||
reference to the L3 agent, and will register as an observer of events.
|
||||
|
||||
This base class provides a definition for all of the L3 event handlers
|
||||
that a service could "observe". A child class for a service type will
|
||||
implement handlers, for events of interest.
|
||||
"""
|
||||
|
||||
def __init__(self, l3_agent):
|
||||
"""Base class for an advanced service.
|
||||
|
||||
Do not directly instantiate objects of this class. Should only be
|
||||
called indirectly by a child class's instance() invocation.
|
||||
"""
|
||||
self.l3_agent = l3_agent
|
||||
# NOTE: Copying L3 agent attributes, so that they are accessible
|
||||
# from device drivers, which are now provided a service instance.
|
||||
# TODO(pcm): Address this in future refactorings.
|
||||
self.conf = l3_agent.conf
|
||||
|
||||
# NOTE: Handler definitions for events generated by the L3 agent.
|
||||
# Subclasses of AdvancedService can override these to perform service
|
||||
# specific actions. Unique methods are defined for add/update, as
|
||||
# some services may want to take different actions.
|
||||
def before_router_added(self, ri):
|
||||
"""Actions taken before router_info created."""
|
||||
pass
|
||||
|
||||
def after_router_added(self, ri):
|
||||
"""Actions taken after router_info created."""
|
||||
pass
|
||||
|
||||
def before_router_updated(self, ri):
|
||||
"""Actions before processing for an updated router."""
|
||||
pass
|
||||
|
||||
def after_router_updated(self, ri):
|
||||
"""Actions add processing for an updated router."""
|
||||
pass
|
||||
|
||||
def before_router_removed(self, ri):
|
||||
"""Actions before removing router."""
|
||||
pass
|
||||
|
||||
def after_router_removed(self, ri):
|
||||
"""Actions after processing and removing router."""
|
||||
pass
|
||||
|
|
@ -36,13 +36,14 @@ from neutron.agent.linux import dhcp
|
|||
from neutron.agent.linux import external_process
|
||||
from neutron.agent.linux import ip_lib
|
||||
from neutron.agent.linux import utils
|
||||
from neutron.callbacks import events
|
||||
from neutron.callbacks import manager
|
||||
from neutron.callbacks import registry
|
||||
from neutron.callbacks import resources
|
||||
from neutron.common import config as common_config
|
||||
from neutron.common import constants as l3_constants
|
||||
from neutron.common import utils as common_utils
|
||||
from neutron.openstack.common import uuidutils
|
||||
from neutron.services import advanced_service as adv_svc
|
||||
from neutron.tests.common import net_helpers
|
||||
from neutron.tests.functional.agent.linux import base
|
||||
from neutron.tests.functional.agent.linux import helpers
|
||||
|
|
@ -309,11 +310,6 @@ class L3AgentTestFramework(base.BaseOVSLinuxTestCase):
|
|||
|
||||
|
||||
class L3AgentTestCase(L3AgentTestFramework):
|
||||
def test_observer_notifications_legacy_router(self):
|
||||
self._test_observer_notifications(enable_ha=False)
|
||||
|
||||
def test_observer_notifications_ha_router(self):
|
||||
self._test_observer_notifications(enable_ha=True)
|
||||
|
||||
def test_keepalived_state_change_notification(self):
|
||||
enqueue_mock = mock.patch.object(
|
||||
|
|
@ -356,24 +352,40 @@ class L3AgentTestCase(L3AgentTestFramework):
|
|||
lambda: self._expected_rpc_report(
|
||||
{router1.router_id: 'standby', router2.router_id: 'active'}))
|
||||
|
||||
def _test_observer_notifications(self, enable_ha):
|
||||
"""Test create, update, delete of router and notifications."""
|
||||
with mock.patch.object(
|
||||
self.agent.event_observers, 'notify') as notify:
|
||||
router_info = self.generate_router_info(enable_ha)
|
||||
router = self.manage_router(self.agent, router_info)
|
||||
self.agent._process_updated_router(router.router)
|
||||
self._delete_router(self.agent, router.router_id)
|
||||
def test_agent_notifications_for_router_events(self):
|
||||
"""Test notifications for router create, update, and delete.
|
||||
|
||||
calls = notify.call_args_list
|
||||
self.assertEqual(
|
||||
[((adv_svc.AdvancedService.before_router_added, router),),
|
||||
((adv_svc.AdvancedService.after_router_added, router),),
|
||||
((adv_svc.AdvancedService.before_router_updated, router),),
|
||||
((adv_svc.AdvancedService.after_router_updated, router),),
|
||||
((adv_svc.AdvancedService.before_router_removed, router),),
|
||||
((adv_svc.AdvancedService.after_router_removed, router),)],
|
||||
calls)
|
||||
Make sure that when the agent sends notifications of router events
|
||||
for router create, update, and delete, that the correct handler is
|
||||
called with the right resource, event, and router information.
|
||||
"""
|
||||
event_handler = mock.Mock()
|
||||
registry.subscribe(event_handler,
|
||||
resources.ROUTER, events.BEFORE_CREATE)
|
||||
registry.subscribe(event_handler,
|
||||
resources.ROUTER, events.AFTER_CREATE)
|
||||
registry.subscribe(event_handler,
|
||||
resources.ROUTER, events.BEFORE_UPDATE)
|
||||
registry.subscribe(event_handler,
|
||||
resources.ROUTER, events.AFTER_UPDATE)
|
||||
registry.subscribe(event_handler,
|
||||
resources.ROUTER, events.BEFORE_DELETE)
|
||||
registry.subscribe(event_handler,
|
||||
resources.ROUTER, events.AFTER_DELETE)
|
||||
|
||||
router_info = self.generate_router_info(enable_ha=False)
|
||||
router = self.manage_router(self.agent, router_info)
|
||||
self.agent._process_updated_router(router.router)
|
||||
self._delete_router(self.agent, router.router_id)
|
||||
|
||||
expected_calls = [
|
||||
mock.call('router', 'before_create', self.agent, router=router),
|
||||
mock.call('router', 'after_create', self.agent, router=router),
|
||||
mock.call('router', 'before_update', self.agent, router=router),
|
||||
mock.call('router', 'after_update', self.agent, router=router),
|
||||
mock.call('router', 'before_delete', self.agent, router=router),
|
||||
mock.call('router', 'after_delete', self.agent, router=router)]
|
||||
event_handler.assert_has_calls(expected_calls)
|
||||
|
||||
def test_legacy_router_lifecycle(self):
|
||||
self._router_lifecycle(enable_ha=False, dual_stack=True)
|
||||
|
|
|
|||
|
|
@ -1,77 +0,0 @@
|
|||
# Copyright 2014 OpenStack Foundation.
|
||||
# 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
|
||||
import testtools
|
||||
|
||||
from neutron.agent.l3 import event_observers
|
||||
from neutron.services import advanced_service as adv_svc
|
||||
from neutron.tests import base
|
||||
|
||||
|
||||
class DummyService1(adv_svc.AdvancedService):
|
||||
def before_router_added(self, ri):
|
||||
pass
|
||||
|
||||
def after_router_added(self, ri):
|
||||
pass
|
||||
|
||||
|
||||
class DummyService2(adv_svc.AdvancedService):
|
||||
def before_router_added(self, ri):
|
||||
pass
|
||||
|
||||
|
||||
class TestL3EventObservers(base.BaseTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestL3EventObservers, self).setUp()
|
||||
self.event_observers = event_observers.L3EventObservers()
|
||||
|
||||
def test_add_observer(self):
|
||||
observer = object()
|
||||
self.assertNotIn(observer, self.event_observers.observers)
|
||||
self.event_observers.add(observer)
|
||||
self.assertIn(observer, self.event_observers.observers)
|
||||
|
||||
def test_add_duplicate_observer_type_raises(self):
|
||||
agent = mock.Mock()
|
||||
observer = DummyService1(agent)
|
||||
self.event_observers.add(observer)
|
||||
|
||||
observer2 = DummyService1(agent)
|
||||
with testtools.ExpectedException(ValueError):
|
||||
self.event_observers.add(observer2)
|
||||
|
||||
self.assertEqual(1, len(self.event_observers.observers))
|
||||
|
||||
def test_observers_in_service_notified(self):
|
||||
"""Test that correct handlers for multiple services are called."""
|
||||
l3_agent = mock.Mock()
|
||||
router_info = mock.Mock()
|
||||
observer1 = DummyService1(l3_agent)
|
||||
observer2 = DummyService2(l3_agent)
|
||||
observer1_before_add = mock.patch.object(
|
||||
DummyService1, 'before_router_added').start()
|
||||
observer2_before_add = mock.patch.object(
|
||||
DummyService2, 'before_router_added').start()
|
||||
|
||||
self.event_observers.add(observer1)
|
||||
self.event_observers.add(observer2)
|
||||
self.event_observers.notify(
|
||||
adv_svc.AdvancedService.before_router_added, router_info)
|
||||
|
||||
observer1_before_add.assert_called_with(router_info)
|
||||
observer2_before_add.assert_called_with(router_info)
|
||||
|
|
@ -1,83 +0,0 @@
|
|||
# Copyright 2014 OpenStack Foundation.
|
||||
# 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.l3 import event_observers
|
||||
from neutron.services import advanced_service
|
||||
from neutron.tests import base
|
||||
|
||||
|
||||
class FakeServiceA(advanced_service.AdvancedService):
|
||||
pass
|
||||
|
||||
|
||||
class FakeServiceB(advanced_service.AdvancedService):
|
||||
pass
|
||||
|
||||
|
||||
class TestAdvancedService(base.BaseTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestAdvancedService, self).setUp()
|
||||
self.agent = mock.Mock()
|
||||
self.test_observers = event_observers.L3EventObservers()
|
||||
|
||||
def test_create_service(self):
|
||||
"""Test agent saved and service added to observer list."""
|
||||
my_service = FakeServiceA(self.agent)
|
||||
self.test_observers.add(my_service)
|
||||
self.assertIn(my_service, self.test_observers.observers)
|
||||
self.assertEqual(self.agent, my_service.l3_agent)
|
||||
|
||||
def test_shared_observers_for_different_services(self):
|
||||
"""Test different service type instances created.
|
||||
|
||||
The services are unique instances, with different agents, but
|
||||
sharing the same observer list.
|
||||
"""
|
||||
a = FakeServiceA(self.agent)
|
||||
self.test_observers.add(a)
|
||||
self.assertEqual(self.agent, a.l3_agent)
|
||||
self.assertIn(a, self.test_observers.observers)
|
||||
|
||||
another_agent = mock.Mock()
|
||||
b = FakeServiceB(another_agent)
|
||||
self.test_observers.add(b)
|
||||
self.assertNotEqual(a, b)
|
||||
self.assertEqual(another_agent, b.l3_agent)
|
||||
self.assertIn(b, self.test_observers.observers)
|
||||
self.assertEqual(2, len(self.test_observers.observers))
|
||||
|
||||
def test_unique_observers_for_different_services(self):
|
||||
"""Test different service types with different observer lists.
|
||||
|
||||
The services are unique instances, shared the same agent, but
|
||||
are using different observer lists.
|
||||
"""
|
||||
a = FakeServiceA(self.agent)
|
||||
self.test_observers.add(a)
|
||||
other_observers = event_observers.L3EventObservers()
|
||||
b = FakeServiceB(self.agent)
|
||||
other_observers.add(b)
|
||||
|
||||
self.assertNotEqual(a, b)
|
||||
self.assertEqual(self.agent, a.l3_agent)
|
||||
self.assertIn(a, self.test_observers.observers)
|
||||
self.assertEqual(1, len(self.test_observers.observers))
|
||||
|
||||
self.assertEqual(self.agent, b.l3_agent)
|
||||
self.assertIn(b, other_observers.observers)
|
||||
self.assertEqual(1, len(other_observers.observers))
|
||||
Loading…
Reference in New Issue