From f5f682c63a043d59388731d78fae98481685019e Mon Sep 17 00:00:00 2001
From: Yasuhiro Kimura <kimura.yasuhiro@nttcom.co.jp>
Date: Fri, 29 Jun 2018 14:19:02 +0900
Subject: [PATCH] Modify logic of l3-agent to be notified
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Although no DVR router is hosted on controller node,
neutron-server notifies l3-agent on compute node.
As a result, the following bug happen when VM migrate or resize.
　1) L3-agent on compute node output error logs
     when VM connect no DVR and HA router.
　2) Unused namespace is appeared on compute node
     when VM connect no DVR and no HA router.
So, I modify logic of l3-agent to be notified
after migrating or resizing.

Change-Id: I53955b885f53d4ee15377a08627c2c25cb1b7041
Closes-Bug: #1775310
---
 neutron/db/l3_dvrscheduler_db.py              |  9 ++++-
 .../unit/scheduler/test_l3_agent_scheduler.py | 37 +++++++++++++++++--
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py
index 866c3beaac4..e50f5071edd 100644
--- a/neutron/db/l3_dvrscheduler_db.py
+++ b/neutron/db/l3_dvrscheduler_db.py
@@ -453,8 +453,13 @@ def _notify_l3_agent_port_update(resource, event, trigger, **kwargs):
             fips = l3plugin._get_floatingips_by_port_id(
                 context, port_id=original_port['id'])
             fip = fips[0] if fips else None
-            if fip and not (removed_routers and
-                            fip['router_id'] in removed_routers):
+            removed_router_ids = [
+                info['router_id'] for info in removed_routers
+            ]
+            if (fip and
+                l3plugin.is_distributed_router(fip['router_id']) and
+                not (removed_routers and
+                     fip['router_id'] in removed_router_ids)):
                 l3plugin.l3_rpc_notifier.routers_updated_on_host(
                     context, [fip['router_id']],
                     original_port[portbindings.HOST_ID])
diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
index b2745f73219..97a041e8af0 100644
--- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
+++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
@@ -1036,9 +1036,34 @@ class L3DvrSchedulerTestCase(L3SchedulerBaseMixin,
         fip = {'router_id': 'router_id'}
         self._test__notify_l3_agent_port_binding_change(None, fip)
 
+    def test__notify_l3_agent_port_binding_change_fip_dvr(self):
+        fip = {'router_id': 'router_id'}
+        is_distributed = True
+        self._test__notify_l3_agent_port_binding_change(None,
+                                                        fip, is_distributed)
+
+    def test__notify_l3_agent_port_binding_change_fip_dvr_rmrt(self):
+        fip = {'router_id': 'router_id'}
+        router_to_remove = [{'agent_id': 'foo_agent',
+                             'router_id': 'foo_id',
+                             'host': 'vm-host1'}]
+        is_distributed = True
+        self._test__notify_l3_agent_port_binding_change(router_to_remove,
+                                                        fip, is_distributed)
+
+    def test__notify_l3_agent_port_binding_change_fip_dvr_on_rmrt(self):
+        fip = {'router_id': 'foo_id'}
+        router_to_remove = [{'agent_id': 'foo_agent',
+                             'router_id': 'foo_id',
+                             'host': 'vm-host1'}]
+        is_distributed = True
+        self._test__notify_l3_agent_port_binding_change(router_to_remove,
+                                                        fip, is_distributed)
+
     def _test__notify_l3_agent_port_binding_change(self,
                                                    routers_to_remove=None,
-                                                   fip=None):
+                                                   fip=None,
+                                                   is_distributed=False):
         source_host = 'vm-host1'
         kwargs = {
             'context': self.adminContext,
@@ -1055,9 +1080,12 @@ class L3DvrSchedulerTestCase(L3SchedulerBaseMixin,
         l3plugin = mock.Mock()
         directory.add_plugin(plugin_constants.L3, l3plugin)
         with mock.patch.object(l3plugin, 'get_dvr_routers_to_remove',
-                               return_value=routers_to_remove),\
+                               return_value=routers_to_remove
+                               if routers_to_remove else []),\
                 mock.patch.object(l3plugin, '_get_floatingips_by_port_id',
-                                  return_value=[fip] if fip else []):
+                                  return_value=[fip] if fip else []),\
+                mock.patch.object(l3plugin, 'is_distributed_router',
+                                  return_value=is_distributed):
             l3_dvrscheduler_db._notify_l3_agent_port_update(
                 'port', 'after_update', mock.ANY, **kwargs)
             if routers_to_remove:
@@ -1066,7 +1094,8 @@ class L3DvrSchedulerTestCase(L3SchedulerBaseMixin,
                 self.assertEqual(
                     1,
                     l3plugin.delete_arp_entry_for_dvr_service_port.call_count)
-            if fip and not routers_to_remove:
+            if fip and is_distributed and not (routers_to_remove and
+                fip['router_id'] is routers_to_remove[0]['router_id']):
                 (l3plugin.l3_rpc_notifier.routers_updated_on_host.
                  assert_called_once_with(mock.ANY, ['router_id'], source_host))
             self.assertEqual(