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
This commit is contained in:
parent
4d3038d96e
commit
877778ee4c
|
@ -60,6 +60,18 @@ class DhcpAgentNotifyAPI(object):
|
|||
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 @@ class DhcpAgentNotifyAPI(object):
|
|||
{'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:
|
||||
|
|
|
@ -17,7 +17,6 @@ import collections
|
|||
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 @@ import webob.exc
|
|||
|
||||
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 @@ class Controller(object):
|
|||
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 @@ class Controller(object):
|
|||
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 @@ class Controller(object):
|
|||
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 @@ class Controller(object):
|
|||
{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 @@ class Controller(object):
|
|||
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
|
||||
|
||||
|
|
|
@ -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_READ = 'after_read'
|
|||
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'
|
||||
|
|
|
@ -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 @@ ROUTER_INTERFACE = 'router_interface'
|
|||
SECURITY_GROUP = 'security_group'
|
||||
SECURITY_GROUP_RULE = 'security_group_rule'
|
||||
SUBNET = 'subnet'
|
||||
SUBNETS = 'subnets'
|
||||
SUBNET_GATEWAY = 'subnet_gateway'
|
||||
SUBNETPOOL_ADDRESS_SCOPE = 'subnetpool_address_scope'
|
||||
|
|
|
@ -24,7 +24,6 @@ from oslo_db import exception as db_exc
|
|||
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 @@ import webtest
|
|||
|
||||
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 @@ class NotificationTest(APIv2TestBase):
|
|||
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 @@ class DHCPNotificationTest(APIv2TestBase):
|
|||
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):
|
||||
|
|
|
@ -1464,17 +1464,16 @@ class OvsDhcpAgentNotifierTestCase(test_agent.AgentDBTestMixIn,
|
|||
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
|
||||
|
|
|
@ -2113,8 +2113,7 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
|
|||
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,
|
||||
notify.assert_any_call(resources.FLOATING_IP,
|
||||
events.AFTER_UPDATE,
|
||||
mock.ANY,
|
||||
context=mock.ANY,
|
||||
|
@ -2142,8 +2141,7 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
|
|||
self._update('floatingips',
|
||||
fip['floatingip']['id'],
|
||||
{'floatingip': {'port_id': None}})
|
||||
notify.assert_called_once_with(
|
||||
resources.FLOATING_IP,
|
||||
notify.assert_any_call(resources.FLOATING_IP,
|
||||
events.AFTER_UPDATE,
|
||||
mock.ANY,
|
||||
context=mock.ANY,
|
||||
|
|
Loading…
Reference in New Issue