From fad444ff089d7c48b2918401dbbc283265ba0c78 Mon Sep 17 00:00:00 2001 From: Maru Newby Date: Tue, 10 Dec 2013 16:10:42 +0000 Subject: [PATCH] Send DHCP notifications regardless of agent status The Neutron service, when under load, may not be able to process agent heartbeats in a timely fashion. This can result in agents being erroneously considered inactive. Previously, DHCP notifications for which active agents could not be found were silently dropped. This change ensures that notifications for a given network are sent to agents even if those agents do not appear to be active. Additionally, if no enabled dhcp agents can be found for a given network, an error will be logged. Raising an exception might be preferable, but has such a large testing impact that it will be submitted as a separate patch if deemed necessary. Partial-bug: #1192381 Change-Id: Id3e639d9cf3d16708fd66a4baebd3fbeeed3dde8 --- .../rpc/agentnotifiers/dhcp_rpc_agent_api.py | 35 +++++++-- neutron/db/agents_db.py | 4 + neutron/tests/unit/api/__init__.py | 0 neutron/tests/unit/api/rpc/__init__.py | 0 .../unit/api/rpc/agentnotifiers/__init__.py | 0 .../agentnotifiers/test_dhcp_rpc_agent_api.py | 76 +++++++++++++++++++ 6 files changed, 108 insertions(+), 7 deletions(-) create mode 100644 neutron/tests/unit/api/__init__.py create mode 100644 neutron/tests/unit/api/rpc/__init__.py create mode 100644 neutron/tests/unit/api/rpc/agentnotifiers/__init__.py create mode 100644 neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py diff --git a/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py b/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py index 1086a9e58b..4ed724dcb3 100644 --- a/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py +++ b/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py @@ -43,12 +43,11 @@ class DhcpAgentNotifyAPI(proxy.RpcProxy): super(DhcpAgentNotifyAPI, self).__init__( topic=topic, default_version=self.BASE_RPC_API_VERSION) - def _get_dhcp_agents(self, context, network_id): + def _get_enabled_dhcp_agents(self, context, network_id): + """Return enabled dhcp agents associated with the given network.""" plugin = manager.NeutronManager.get_plugin() - dhcp_agents = plugin.get_dhcp_agents_hosting_networks( - context, [network_id], active=True) - return [(dhcp_agent.host, dhcp_agent.topic) for - dhcp_agent in dhcp_agents] + agents = plugin.get_dhcp_agents_hosting_networks(context, [network_id]) + return [x for x in agents if x.admin_state_up] def _notification_host(self, context, method, payload, host): """Notify the agent on host.""" @@ -76,11 +75,33 @@ class DhcpAgentNotifyAPI(proxy.RpcProxy): context, 'network_create_end', {'network': {'id': network_id}}, agent['host']) - for (host, topic) in self._get_dhcp_agents(context, network_id): + agents = self._get_enabled_dhcp_agents(context, network_id) + if not agents: + LOG.error(_("No DHCP agents are associated with network " + "'%(net_id)s'. Unable to send notification " + "for '%(method)s' with payload: %(payload)s"), + { + 'net_id': network_id, + 'method': method, + 'payload': payload, + }) + return + active_agents = [x for x in agents if x.is_active] + if active_agents != agents: + LOG.warning(_("Only %(active)d of %(total)d DHCP agents " + "associated with network '%(net_id)s' are " + "marked as active, so notifications may " + "be sent to inactive agents."), + { + 'active': len(active_agents), + 'total': len(agents), + 'net_id': network_id, + }) + for agent in agents: self.cast( context, self.make_msg(method, payload=payload), - topic='%s.%s' % (topic, host)) + topic='%s.%s' % (agent.topic, agent.host)) else: # besides the non-agentscheduler plugin, # There is no way to query who is hosting the network diff --git a/neutron/db/agents_db.py b/neutron/db/agents_db.py index 9a574a3f27..960085f97e 100644 --- a/neutron/db/agents_db.py +++ b/neutron/db/agents_db.py @@ -66,6 +66,10 @@ class Agent(model_base.BASEV2, models_v2.HasId): # configurations: a json dict string, I think 4095 is enough configurations = sa.Column(sa.String(4095), nullable=False) + @property + def is_active(self): + return not AgentDbMixin.is_agent_down(self.heartbeat_timestamp) + class AgentDbMixin(ext_agent.AgentPluginBase): """Mixin class to add agent extension to db_plugin_base_v2.""" diff --git a/neutron/tests/unit/api/__init__.py b/neutron/tests/unit/api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/neutron/tests/unit/api/rpc/__init__.py b/neutron/tests/unit/api/rpc/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/neutron/tests/unit/api/rpc/agentnotifiers/__init__.py b/neutron/tests/unit/api/rpc/agentnotifiers/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py b/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py new file mode 100644 index 0000000000..b175d34036 --- /dev/null +++ b/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py @@ -0,0 +1,76 @@ +# Copyright (c) 2013 Red Hat, Inc. +# +# 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 contextlib + +import mock + +from neutron.api.rpc.agentnotifiers import dhcp_rpc_agent_api +from neutron.common import utils +from neutron import manager +from neutron.tests import base + + +class TestDhcpAgentNotifyAPI(base.BaseTestCase): + + def setUp(self): + super(TestDhcpAgentNotifyAPI, self).setUp() + self.notify = dhcp_rpc_agent_api.DhcpAgentNotifyAPI() + + def test_get_enabled_dhcp_agents_filters_disabled_agents(self): + disabled_agent = mock.Mock() + disabled_agent.admin_state_up = False + enabled_agent = mock.Mock() + with mock.patch.object(manager.NeutronManager, + 'get_plugin') as mock_get_plugin: + mock_get_plugin.return_value = mock_plugin = mock.Mock() + with mock.patch.object( + mock_plugin, 'get_dhcp_agents_hosting_networks' + ) as mock_get_agents: + mock_get_agents.return_value = [disabled_agent, enabled_agent] + result = self.notify._get_enabled_dhcp_agents('ctx', 'net_id') + self.assertEqual(result, [enabled_agent]) + + def _test_notification(self, agents): + with contextlib.nested( + mock.patch.object(manager.NeutronManager, 'get_plugin'), + mock.patch.object(utils, 'is_extension_supported'), + mock.patch.object(self.notify, '_get_enabled_dhcp_agents') + ) as (m1, m2, mock_get_agents): + mock_get_agents.return_value = agents + self.notify._notification(mock.Mock(), 'foo', {}, 'net_id') + + def test_notification_sends_cast_for_enabled_agent(self): + with mock.patch.object(self.notify, 'cast') as mock_cast: + self._test_notification([mock.Mock()]) + self.assertEqual(mock_cast.call_count, 1) + + def test_notification_logs_error_for_no_enabled_agents(self): + with mock.patch.object(self.notify, 'cast') as mock_cast: + with mock.patch.object(dhcp_rpc_agent_api.LOG, + 'error') as mock_log: + self._test_notification([]) + self.assertEqual(mock_cast.call_count, 0) + self.assertEqual(mock_log.call_count, 1) + + def test_notification_logs_warning_for_inactive_agents(self): + agent = mock.Mock() + agent.is_active = False + with mock.patch.object(self.notify, 'cast') as mock_cast: + with mock.patch.object(dhcp_rpc_agent_api.LOG, + 'warning') as mock_log: + self._test_notification([agent]) + self.assertEqual(mock_cast.call_count, 1) + self.assertEqual(mock_log.call_count, 1)