]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Revert "DVR: Notify specific agent when update floatingip"
authorIhar Hrachyshka <ihrachys@redhat.com>
Mon, 19 Oct 2015 12:44:58 +0000 (12:44 +0000)
committerIhar Hrachyshka <ihrachys@redhat.com>
Mon, 19 Oct 2015 13:54:11 +0000 (13:54 +0000)
This reverts commit 52e91f48f2327b47f126893f9cb12f153380a9a6.

The patch broke notifications about FIP updates and triggered 100%
gate failures for Ironic gate.

I believe that I0cbe8c51c3714e6cbdc48ca37135b783f8014905 is also
breaking notifications, but for FIP create, which probably was not
utilized in any gate before and hence not caught in time.

The change the reverted patch introduced made update_floatingip to
fetch router based on FIP router_id field on every call, which was
not the case before the patch. For some reason unknown at the
moment, we get NotFound from database on this fetch.

The patch does not answer the question why we get NotFound from
database on fetching a FIP router_id, but that's another issue that
should be investigated while Ironic gate is happy.

Change-Id: I4affac49d7c63f47c5654b94b28f4cb7471e87b0
Closes-Bug: #1507558
Related-Bug: #1507602

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

index 6522e94b4117041a8c98697070ca8a57a56f054f..ce6263e2e4bfafe134d75dd917417ff3cb3c71b3 100644 (file)
@@ -685,24 +685,11 @@ 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)
-        self._notify_floating_ip_change(context, floating_ip)
-        return floating_ip
-
-    def _notify_floating_ip_change(self, context, floating_ip):
         router_id = floating_ip['router_id']
         if not router_id:
-            return
+            return floating_ip
 
-        try:
-            router = self._get_router(context, router_id)
-        except l3.RouterNotFound:
-            # TODO(changzhi): this is present to avoid grenade failing on a
-            # race condition(<bug:1486828>). Can be removed when underlying
-            # race condition is cleaned up.
-            LOG.debug("Router %(router_id)s not found. "
-                      "Just ingore this router. ",
-                      {"router_id": router_id})
-            return
+        router = self._get_router(context, router_id)
         # we need to notify agents only in case Floating IP is associated
         fixed_port_id = floating_ip['port_id']
         if fixed_port_id:
@@ -711,16 +698,9 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
                 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
+                self.notify_router_updated(context, router_id,
+                                           'create_floatingip')
+        return floating_ip
 
 
 def is_distributed_router(router):
index 1cc8ac340655ac5df98e3f18c6b99cf6e3116eda..75d5ce25a4cdd4f35a1e65bca2f983a496e3f163 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']], None)
+                        self.context, [router['id']], 'create_floatingip')
                     self.assertFalse(
                         l3_notif.routers_updated_on_host.called)
 
@@ -266,82 +266,3 @@ 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)