]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
DVR: Notify specific agent when update floatingip
authorchangzhi <changzhi@unitedstack.com>
Thu, 20 Aug 2015 13:40:42 +0000 (21:40 +0800)
committerOleg Bondarev <obondarev@mirantis.com>
Tue, 20 Oct 2015 09:13:26 +0000 (12:13 +0300)
The L3 agent was determined when update floatingip.
So notify the specific agent rather than notify all agents.
This will save some RPC resources. This is only for DVR routers.
Legacy and HA routers notify only the relevant agents.

This reproposes commit 52e91f48f2327b47f126893f9cb12f153380a9a6
which was reverted by commit a2f7e0343a147a30a637af4e1cb9a866f557e87d
because of Ironic gate failures.
Now the patch preserves original behavior for legacy routers and
should not break Ironic tests.

Partial-Bug: #1486828
Related-Bug: #1507602
Change-Id: I4ef7a69ad033b979ea0e29620a4febfe5e0c30dd

neutron/db/l3_dvr_db.py
neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py

index a65b82df4cbe061fad0f30d3402ecb2595a6754f..5cce619cd27aa404fa6d086522a853f68a1648f1 100644 (file)
@@ -32,7 +32,7 @@ from neutron.db import l3_dvrscheduler_db as l3_dvrsched_db
 from neutron.db import models_v2
 from neutron.extensions import l3
 from neutron.extensions import portbindings
-from neutron.i18n import _LI
+from neutron.i18n import _LI, _LW
 from neutron import manager
 from neutron.plugins.common import constants
 from neutron.plugins.common import utils as p_utils
@@ -687,22 +687,43 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
                           initial_status=l3_const.FLOATINGIP_STATUS_ACTIVE):
         floating_ip = self._create_floatingip(
             context, floatingip, initial_status)
-        router_id = floating_ip['router_id']
-        if not router_id:
-            return floating_ip
+        self._notify_floating_ip_change(context, floating_ip)
+        return floating_ip
 
-        router = self._get_router(context, router_id)
-        # we need to notify agents only in case Floating IP is associated
+    def _notify_floating_ip_change(self, context, floating_ip):
+        router_id = floating_ip['router_id']
         fixed_port_id = floating_ip['port_id']
-        if fixed_port_id:
-            if is_distributed_router(router):
-                host = self._get_vm_port_hostid(context, fixed_port_id)
-                self.l3_rpc_notifier.routers_updated_on_host(
-                    context, [router_id], host)
-            else:
-                self.notify_router_updated(context, router_id,
-                                           'create_floatingip')
-        return floating_ip
+        # we need to notify agents only in case Floating IP is associated
+        if not router_id or not fixed_port_id:
+            return
+
+        try:
+            router = self._get_router(context, router_id)
+        except l3.RouterNotFound:
+            # TODO(obondarev): bug 1507602 was filed to investigate race
+            # condition here. For now we preserve original behavior and do
+            # broad notification
+            LOG.warning(_LW("Router %s was not found. "
+                            "Doing broad notification."),
+                        router_id)
+            self.notify_router_updated(context, router_id)
+            return
+
+        if is_distributed_router(router):
+            host = self._get_vm_port_hostid(context, fixed_port_id)
+            self.l3_rpc_notifier.routers_updated_on_host(
+                context, [router_id], host)
+        else:
+            self.notify_router_updated(context, router_id)
+
+    def update_floatingip(self, context, id, floatingip):
+        old_floatingip, floatingip = self._update_floatingip(
+            context, id, floatingip)
+        self._notify_floating_ip_change(context, old_floatingip)
+        if (floatingip['router_id'] != old_floatingip['router_id'] or
+                floatingip['port_id'] != old_floatingip['port_id']):
+            self._notify_floating_ip_change(context, floatingip)
+        return floatingip
 
 
 def is_distributed_router(router):
index 75d5ce25a4cdd4f35a1e65bca2f983a496e3f163..1cc8ac340655ac5df98e3f18c6b99cf6e3116eda 100644 (file)
@@ -257,7 +257,7 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
                     self.assertFalse(l3_notif.routers_updated.called)
                 else:
                     l3_notif.routers_updated.assert_called_once_with(
-                        self.context, [router['id']], 'create_floatingip')
+                        self.context, [router['id']], None)
                     self.assertFalse(
                         l3_notif.routers_updated_on_host.called)
 
@@ -266,3 +266,82 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
 
     def test_create_floating_ip_agent_notification_non_dvr(self):
         self._test_create_floating_ip_agent_notification(dvr=False)
+
+    def _test_update_floating_ip_agent_notification(self, dvr=True):
+        with self.subnet() as ext_subnet,\
+                self.subnet(cidr='20.0.0.0/24') as int_subnet1,\
+                self.subnet(cidr='30.0.0.0/24') as int_subnet2,\
+                self.port(subnet=int_subnet1,
+                          device_owner='compute:None') as int_port1,\
+                self.port(subnet=int_subnet2,
+                          device_owner='compute:None') as int_port2:
+            # locate internal ports on different hosts
+            self.core_plugin.update_port(
+                self.context, int_port1['port']['id'],
+                {'port': {'binding:host_id': 'host1'}})
+            self.core_plugin.update_port(
+                self.context, int_port2['port']['id'],
+                {'port': {'binding:host_id': 'host2'}})
+            # and create l3 agents on corresponding hosts
+            helpers.register_l3_agent(host='host1',
+                agent_mode=l3_const.L3_AGENT_MODE_DVR)
+            helpers.register_l3_agent(host='host2',
+                agent_mode=l3_const.L3_AGENT_MODE_DVR)
+
+            # make net external
+            ext_net_id = ext_subnet['subnet']['network_id']
+            self._update('networks', ext_net_id,
+                     {'network': {external_net.EXTERNAL: True}})
+
+            router1 = self._create_router(distributed=dvr)
+            router2 = self._create_router(distributed=dvr)
+            for router in (router1, router2):
+                self.l3_plugin.update_router(
+                    self.context, router['id'],
+                    {'router': {
+                        'external_gateway_info': {'network_id': ext_net_id}}})
+            self.l3_plugin.add_router_interface(
+                self.context, router1['id'],
+                {'subnet_id': int_subnet1['subnet']['id']})
+            self.l3_plugin.add_router_interface(
+                self.context, router2['id'],
+                {'subnet_id': int_subnet2['subnet']['id']})
+
+            floating_ip = {'floating_network_id': ext_net_id,
+                           'router_id': router1['id'],
+                           'port_id': int_port1['port']['id'],
+                           'tenant_id': int_port1['port']['tenant_id']}
+            floating_ip = self.l3_plugin.create_floatingip(
+                self.context, {'floatingip': floating_ip})
+
+            with mock.patch.object(
+                    self.l3_plugin, '_l3_rpc_notifier') as l3_notif:
+                updated_floating_ip = {'router_id': router2['id'],
+                                       'port_id': int_port2['port']['id']}
+                self.l3_plugin.update_floatingip(
+                    self.context, floating_ip['id'],
+                    {'floatingip': updated_floating_ip})
+                if dvr:
+                    self.assertEqual(
+                        2, l3_notif.routers_updated_on_host.call_count)
+                    expected_calls = [
+                        mock.call(self.context, [router1['id']], 'host1'),
+                        mock.call(self.context, [router2['id']], 'host2')]
+                    l3_notif.routers_updated_on_host.assert_has_calls(
+                        expected_calls)
+                    self.assertFalse(l3_notif.routers_updated.called)
+                else:
+                    self.assertEqual(
+                        2, l3_notif.routers_updated.call_count)
+                    expected_calls = [
+                        mock.call(self.context, [router1['id']], None),
+                        mock.call(self.context, [router2['id']], None)]
+                    l3_notif.routers_updated.assert_has_calls(
+                        expected_calls)
+                    self.assertFalse(l3_notif.routers_updated_on_host.called)
+
+    def test_update_floating_ip_agent_notification(self):
+        self._test_update_floating_ip_agent_notification()
+
+    def test_update_floating_ip_agent_notification_non_dvr(self):
+        self._test_update_floating_ip_agent_notification(dvr=False)