From 877778ee4c7f0e83b54f54b2d7bafec98f89626c Mon Sep 17 00:00:00 2001 From: Armando Migliaccio Date: Sat, 18 Jun 2016 08:19:03 -0700 Subject: [PATCH] Move DHCP notification logic out of API controller Bug 1591766 unveiled an issue where calling the plugin API does not trigger DHCP notifications. This is required by the auto-allocated-topology service plugin that calls core_plugin.update_network(), and expect notifications to be sent out on state changes. To accomplish this, the logic has been encapsulated in the DHCP module, and leveraged via callback mechanisms. For this reason, new events have been introduced, AFTER_REQUEST, and BEFORE_RESPONSE. The latter in particular is the one needed to hook up dhcp notifications in order to preserve backward compatibility. More precisely, core plugins that use DHCP as is or implement their own, (with or without an agent) should already instantiate their own notifier, and if they do not, this should be rectified. A search on codesearch.openstack.org reveals that out-of-tree plugins already specify their own notifiers, and the default initialization is clearly redundant now. Related-bug: #1591766 Change-Id: I7440becb6d30af7159ecaeba09d7a28eceb71bea --- .../rpc/agentnotifiers/dhcp_rpc_agent_api.py | 23 ++++++++++ neutron/api/v2/base.py | 38 +++++---------- neutron/callbacks/events.py | 7 ++- neutron/callbacks/resources.py | 4 ++ neutron/tests/unit/api/v2/test_base.py | 38 ++++++--------- .../tests/unit/db/test_agentschedulers_db.py | 13 +++--- neutron/tests/unit/extensions/test_l3.py | 46 +++++++++---------- 7 files changed, 87 insertions(+), 82 deletions(-) diff --git a/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py b/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py index 795b522afd0..1089ecd39f0 100644 --- a/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py +++ b/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py @@ -60,6 +60,18 @@ def __init__(self, topic=topics.DHCP_AGENT, plugin=None): resources.ROUTER_INTERFACE, events.AFTER_CREATE) registry.subscribe(self._after_router_interface_deleted, resources.ROUTER_INTERFACE, events.AFTER_DELETE) + # register callbacks for events pertaining resources affecting DHCP + callback_resources = ( + resources.NETWORK, + resources.NETWORKS, + resources.PORT, + resources.PORTS, + resources.SUBNET, + resources.SUBNETS, + ) + for resource in callback_resources: + registry.subscribe(self._send_dhcp_notification, + resource, events.BEFORE_RESPONSE) @property def plugin(self): @@ -192,6 +204,17 @@ def _after_router_interface_deleted(self, resource, event, trigger, {'port_id': kwargs['port']['id']}, kwargs['port']['network_id']) + def _send_dhcp_notification(self, resource, event, trigger, context=None, + data=None, method_name=None, collection=None, + **kwargs): + if cfg.CONF.dhcp_agent_notification: + if collection and collection in data: + for body in data[collection]: + item = {resource: body} + self.notify(context, item, method_name) + else: + self.notify(context, data, method_name) + def notify(self, context, data, method_name): # data is {'key' : 'value'} with only one key if method_name not in self.VALID_METHOD_NAMES: diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py index a3e4a7e68da..ef96783b6a6 100644 --- a/neutron/api/v2/base.py +++ b/neutron/api/v2/base.py @@ -17,7 +17,6 @@ import copy import netaddr -from neutron_lib import constants from neutron_lib import exceptions from oslo_config import cfg from oslo_log import log as logging @@ -28,9 +27,10 @@ from neutron._i18n import _, _LE, _LI from neutron.api import api_common -from neutron.api.rpc.agentnotifiers import dhcp_rpc_agent_api from neutron.api.v2 import attributes from neutron.api.v2 import resource as wsgi_resource +from neutron.callbacks import events +from neutron.callbacks import registry from neutron.common import constants as n_const from neutron.common import exceptions as n_exc from neutron.common import rpc as n_rpc @@ -90,12 +90,6 @@ def __init__(self, plugin, collection, resource, attr_info, self._policy_attrs = [name for (name, info) in self._attr_info.items() if info.get('required_by_policy')] self._notifier = n_rpc.get_notifier('network') - # use plugin's dhcp notifier, if this is already instantiated - agent_notifiers = getattr(plugin, 'agent_notifiers', {}) - self._dhcp_agent_notifier = ( - agent_notifiers.get(constants.AGENT_TYPE_DHCP) or - dhcp_rpc_agent_api.DhcpAgentNotifyAPI() - ) if cfg.CONF.notify_nova_on_port_data_changes: from neutron.notifiers import nova self._nova_notifier = nova.Notifier() @@ -333,15 +327,6 @@ def _item(self, request, id, do_authz=False, field_list=None, pluralized=self._collection) return obj - def _send_dhcp_notification(self, context, data, methodname): - if cfg.CONF.dhcp_agent_notification: - if self._collection in data: - for body in data[self._collection]: - item = {self._resource: body} - self._dhcp_agent_notifier.notify(context, item, methodname) - else: - self._dhcp_agent_notifier.notify(context, data, methodname) - def _send_nova_notification(self, action, orig, returned): if hasattr(self, '_nova_notifier'): self._nova_notifier.send_network_change(action, orig, returned) @@ -485,9 +470,10 @@ def notify(create_result): self._notifier.info(request.context, notifier_method, create_result) - self._send_dhcp_notification(request.context, - create_result, - notifier_method) + registry.notify(self._resource, events.BEFORE_RESPONSE, self, + context=request.context, data=create_result, + method_name=notifier_method, + collection=self._collection) return create_result def do_create(body, bulk=False, emulated=False): @@ -578,9 +564,9 @@ def _delete(self, request, id, **kwargs): {self._resource + '_id': id}) result = {self._resource: self._view(request.context, obj)} self._send_nova_notification(action, {}, result) - self._send_dhcp_notification(request.context, - result, - notifier_method) + registry.notify(self._resource, events.BEFORE_RESPONSE, self, + context=request.context, data=result, + method_name=notifier_method) def update(self, request, id, body=None, **kwargs): """Updates the specified entity's attributes.""" @@ -649,9 +635,9 @@ def _update(self, request, id, body, **kwargs): result = {self._resource: self._view(request.context, obj)} notifier_method = self._resource + '.update.end' self._notifier.info(request.context, notifier_method, result) - self._send_dhcp_notification(request.context, - result, - notifier_method) + registry.notify(self._resource, events.BEFORE_RESPONSE, self, + context=request.context, data=result, + method_name=notifier_method) self._send_nova_notification(action, orig_object_copy, result) return result diff --git a/neutron/callbacks/events.py b/neutron/callbacks/events.py index 5b3209a7d23..dbf64a7c14e 100644 --- a/neutron/callbacks/events.py +++ b/neutron/callbacks/events.py @@ -10,7 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. -# String literals representing core events. +# String literals representing events associated to data store operations BEFORE_CREATE = 'before_create' BEFORE_READ = 'before_read' BEFORE_UPDATE = 'before_update' @@ -25,6 +25,11 @@ AFTER_UPDATE = 'after_update' AFTER_DELETE = 'after_delete' +# String literals representing events associated to API operations +BEFORE_RESPONSE = 'before_response' +AFTER_REQUEST = 'after_request' + +# String literals representing events associated to error conditions ABORT_CREATE = 'abort_create' ABORT_READ = 'abort_read' ABORT_UPDATE = 'abort_update' diff --git a/neutron/callbacks/resources.py b/neutron/callbacks/resources.py index 753d7a51151..410809dc9c7 100644 --- a/neutron/callbacks/resources.py +++ b/neutron/callbacks/resources.py @@ -14,7 +14,10 @@ AGENT = 'agent' EXTERNAL_NETWORK = 'external_network' FLOATING_IP = 'floating_ip' +NETWORK = 'network' +NETWORKS = 'networks' PORT = 'port' +PORTS = 'ports' PROCESS = 'process' ROUTER = 'router' ROUTER_GATEWAY = 'router_gateway' @@ -22,5 +25,6 @@ SECURITY_GROUP = 'security_group' SECURITY_GROUP_RULE = 'security_group_rule' SUBNET = 'subnet' +SUBNETS = 'subnets' SUBNET_GATEWAY = 'subnet_gateway' SUBNETPOOL_ADDRESS_SCOPE = 'subnetpool_address_scope' diff --git a/neutron/tests/unit/api/v2/test_base.py b/neutron/tests/unit/api/v2/test_base.py index 4d6a0abad50..7ac7c79ad64 100644 --- a/neutron/tests/unit/api/v2/test_base.py +++ b/neutron/tests/unit/api/v2/test_base.py @@ -24,7 +24,6 @@ from oslo_policy import policy as oslo_policy from oslo_utils import uuidutils import six -from six import moves import six.moves.urllib.parse as urlparse import webob from webob import exc @@ -32,10 +31,10 @@ from neutron.api import api_common from neutron.api import extensions -from neutron.api.rpc.agentnotifiers import dhcp_rpc_agent_api from neutron.api.v2 import attributes from neutron.api.v2 import base as v2_base from neutron.api.v2 import router +from neutron.callbacks import registry from neutron import context from neutron import manager from neutron import policy @@ -1343,20 +1342,19 @@ def test_network_update_notifer(self): self._resource_op_notifier('update', 'network') -class DHCPNotificationTest(APIv2TestBase): +class RegistryNotificationTest(APIv2TestBase): def setUp(self): # This test does not have database support so tracking cannot be used cfg.CONF.set_override('track_quota_usage', False, group='QUOTAS') - super(DHCPNotificationTest, self).setUp() + super(RegistryNotificationTest, self).setUp() - def _test_dhcp_notifier(self, opname, resource, initial_input=None): + def _test_registry_notify(self, opname, resource, initial_input=None): instance = self.plugin.return_value instance.get_networks.return_value = initial_input instance.get_networks_count.return_value = 0 expected_code = exc.HTTPCreated.code - with mock.patch.object(dhcp_rpc_agent_api.DhcpAgentNotifyAPI, - 'notify') as dhcp_notifier: + with mock.patch.object(registry, 'notify') as notify: if opname == 'create': res = self.api.post_json( _get_path('networks'), @@ -1369,35 +1367,27 @@ def _test_dhcp_notifier(self, opname, resource, initial_input=None): if opname == 'delete': res = self.api.delete(_get_path('networks', id=_uuid())) expected_code = exc.HTTPNoContent.code - expected_item = mock.call(mock.ANY, mock.ANY, - resource + "." + opname + ".end") - if initial_input and resource not in initial_input: - resource += 's' - num = len(initial_input[resource]) if initial_input and isinstance( - initial_input[resource], list) else 1 - expected = [expected_item for x in moves.range(num)] - self.assertEqual(expected, dhcp_notifier.call_args_list) - self.assertEqual(num, dhcp_notifier.call_count) + self.assertTrue(notify.called) self.assertEqual(expected_code, res.status_int) - def test_network_create_dhcp_notifer(self): + def test_network_create_registry_notify(self): input = {'network': {'name': 'net', 'tenant_id': _uuid()}} - self._test_dhcp_notifier('create', 'network', input) + self._test_registry_notify('create', 'network', input) - def test_network_delete_dhcp_notifer(self): - self._test_dhcp_notifier('delete', 'network') + def test_network_delete_registry_notify(self): + self._test_registry_notify('delete', 'network') - def test_network_update_dhcp_notifer(self): + def test_network_update_registry_notify(self): input = {'network': {'name': 'net'}} - self._test_dhcp_notifier('update', 'network', input) + self._test_registry_notify('update', 'network', input) - def test_networks_create_bulk_dhcp_notifer(self): + def test_networks_create_bulk_registry_notify(self): input = {'networks': [{'name': 'net1', 'tenant_id': _uuid()}, {'name': 'net2', 'tenant_id': _uuid()}]} - self._test_dhcp_notifier('create', 'network', input) + self._test_registry_notify('create', 'network', input) class QuotaTest(APIv2TestBase): diff --git a/neutron/tests/unit/db/test_agentschedulers_db.py b/neutron/tests/unit/db/test_agentschedulers_db.py index b68a1f01bba..9d0ddb0e802 100644 --- a/neutron/tests/unit/db/test_agentschedulers_db.py +++ b/neutron/tests/unit/db/test_agentschedulers_db.py @@ -1464,17 +1464,16 @@ def test_network_ha_port_create_notification(self): self.assertIn(expected, self.dhcp_notifier_cast.call_args_list) def _is_schedule_network_called(self, device_id): + dhcp_notifier_schedule = mock.patch( + 'neutron.api.rpc.agentnotifiers.dhcp_rpc_agent_api.' + 'DhcpAgentNotifyAPI._schedule_network').start() plugin = manager.NeutronManager.get_plugin() - notifier = plugin.agent_notifiers[constants.AGENT_TYPE_DHCP] with self.subnet() as subnet,\ + self.port(subnet=subnet, device_id=device_id),\ mock.patch.object(plugin, 'get_dhcp_agents_hosting_networks', - return_value=[]),\ - mock.patch.object(notifier, - '_schedule_network', - return_value=[]) as mock_sched: - with self.port(subnet=subnet, device_id=device_id): - return mock_sched.called + return_value=[]): + return dhcp_notifier_schedule.call_count > 1 def test_reserved_dhcp_port_creation(self): device_id = n_const.DEVICE_ID_RESERVED_DHCP_PORT diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index a562fbfb2e7..95989a364b7 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -2113,18 +2113,17 @@ def test_first_floatingip_associate_notification(self): body = self._show('routers', router_id) ext_gw_info = body['router']['external_gateway_info'] ext_fixed_ip = ext_gw_info['external_fixed_ips'][0] - notify.assert_called_once_with( - resources.FLOATING_IP, - events.AFTER_UPDATE, - mock.ANY, - context=mock.ANY, - fixed_ip_address=ip_address, - fixed_port_id=port_id, - floating_ip_address=fip_addr, - floating_network_id=fip_network_id, - last_known_router_id=None, - router_id=router_id, - next_hop=ext_fixed_ip['ip_address']) + notify.assert_any_call(resources.FLOATING_IP, + events.AFTER_UPDATE, + mock.ANY, + context=mock.ANY, + fixed_ip_address=ip_address, + fixed_port_id=port_id, + floating_ip_address=fip_addr, + floating_network_id=fip_network_id, + last_known_router_id=None, + router_id=router_id, + next_hop=ext_fixed_ip['ip_address']) def test_floatingip_disassociate_notification(self): with self.port() as p: @@ -2142,18 +2141,17 @@ def test_floatingip_disassociate_notification(self): self._update('floatingips', fip['floatingip']['id'], {'floatingip': {'port_id': None}}) - notify.assert_called_once_with( - resources.FLOATING_IP, - events.AFTER_UPDATE, - mock.ANY, - context=mock.ANY, - fixed_ip_address=None, - fixed_port_id=None, - floating_ip_address=fip_addr, - floating_network_id=fip_network_id, - last_known_router_id=router_id, - router_id=None, - next_hop=None) + notify.assert_any_call(resources.FLOATING_IP, + events.AFTER_UPDATE, + mock.ANY, + context=mock.ANY, + fixed_ip_address=None, + fixed_port_id=None, + floating_ip_address=fip_addr, + floating_network_id=fip_network_id, + last_known_router_id=router_id, + router_id=None, + next_hop=None) def test_floatingip_association_on_unowned_router(self): # create a router owned by one tenant and associate the FIP with a