DVR:Fix _notify_l3_agent_new_port for proper arp update

Now with notifications coming from ml2 plugin on port create
and port update, it is worth fixing the existing _notify_
l3_agent_new_port for proper arp update and router scheduling.

Previously we have been sending arp update and calling router
scheduling for every update notification for service ports,
but now we can take necessary action only when required, since
the fix to update the arp and router scheduling was recently
done by sending the port info for every new port created.

When _notify_l3_agent_port_update is triggered, we check if the
original port host binding exists and if there is a change in
host binding with respect to the new port, then we go ahead and
reschedule the router on the new host and flush the arp entry.

Related-Bug: #1524020

Change-Id: Ifda623d5413b72bf80f38fba5c12a05a88bb7de5

diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py
index a70b0e49525825a813428f799c1dced290c6ffd0..449270cef65cd3e0e863707dbb8217d83e5b5dba 100644 (file)
--- a/neutron/db/l3_dvrscheduler_db.py
+++ b/neutron/db/l3_dvrscheduler_db.py
@@ -463,15 +463,12 @@ def _notify_l3_agent_new_port(resource, event, trigger, **kwargs):
     if not port:
         return
 
-    l3plugin = manager.NeutronManager.get_service_plugins().get(
-        service_constants.L3_ROUTER_NAT)
-    mac_address_updated = kwargs.get('mac_address_updated')
-    update_device_up = kwargs.get('update_device_up')
-    context = kwargs['context']
-    if mac_address_updated or update_device_up:
-        l3plugin.dvr_vmarp_table_update(context, port, "add")
     if n_utils.is_dvr_serviced(port['device_owner']):
+        l3plugin = manager.NeutronManager.get_service_plugins().get(
+            service_constants.L3_ROUTER_NAT)
+        context = kwargs['context']
         l3plugin.dvr_update_router_addvm(context, port)
+        l3plugin.dvr_vmarp_table_update(context, port, "add")
 
 
 def _notify_port_delete(event, resource, trigger, **kwargs):
@@ -493,6 +490,9 @@ def _notify_l3_agent_port_update(resource, event, trigger, **kwargs):
     if new_port and original_port:
         original_device_owner = original_port.get('device_owner', '')
         new_device_owner = new_port.get('device_owner', '')
+        l3plugin = manager.NeutronManager.get_service_plugins().get(
+                service_constants.L3_ROUTER_NAT)
+        context = kwargs['context']
         is_port_no_longer_serviced = (
             n_utils.is_dvr_serviced(original_device_owner) and
             not n_utils.is_dvr_serviced(new_device_owner))
@@ -501,9 +501,6 @@ def _notify_l3_agent_port_update(resource, event, trigger, **kwargs):
             original_port[portbindings.HOST_ID] !=
             new_port[portbindings.HOST_ID])
         if is_port_no_longer_serviced or is_port_moved:
-            l3plugin = manager.NeutronManager.get_service_plugins().get(
-                service_constants.L3_ROUTER_NAT)
-            context = kwargs['context']
             removed_routers = l3plugin.dvr_deletens_if_no_port(
                 context,
                 original_port['id'],
@@ -518,8 +515,16 @@ def _notify_l3_agent_port_update(resource, event, trigger, **kwargs):
                     event, resource, trigger, **removed_router_args)
             if not n_utils.is_dvr_serviced(new_device_owner):
                 return
-
-    _notify_l3_agent_new_port(resource, event, trigger, **kwargs)
+        is_new_port_binding_changed = (
+            new_port[portbindings.HOST_ID] and
+            (original_port[portbindings.HOST_ID] !=
+                new_port[portbindings.HOST_ID]))
+        if (is_new_port_binding_changed and
+            n_utils.is_dvr_serviced(new_device_owner)):
+            l3plugin.dvr_update_router_addvm(context, new_port)
+            l3plugin.dvr_vmarp_table_update(context, new_port, "add")
+        elif kwargs.get('mac_address_updated'):
+            l3plugin.dvr_vmarp_table_update(context, new_port, "add")
 
 
 def subscribe():

diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
index 32730f457a46f5c752105a9e1c9660b167009de9..0f6e9a564468d05aa0eefcfefbfc170f8ac25693 100644 (file)
--- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
+++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
@@ -970,12 +970,123 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase):
                                return_value={'L3_ROUTER_NAT': l3plugin}):
             l3_dvrscheduler_db._notify_l3_agent_port_update(
                 'port', 'after_update', plugin, **kwargs)
+            self.assertFalse(l3plugin.dvr_vmarp_table_update.called)
+            self.assertFalse(l3plugin.dvr_update_router_addvm.called)
+            self.assertFalse(l3plugin.remove_router_from_l3_agent.called)
+            self.assertFalse(l3plugin.dvr_deletens_if_no_port.called)
+
+    def test__notify_l3_agent_new_port_action(self):
+        kwargs = {
+            'context': self.adminContext,
+            'original_port': None,
+            'port': {
+                'device_owner': DEVICE_OWNER_COMPUTE,
+            },
+        }
+        l3plugin = mock.Mock()
+        with mock.patch.object(manager.NeutronManager,
+                               'get_service_plugins',
+                               return_value={'L3_ROUTER_NAT': l3plugin}):
+            l3_dvrscheduler_db._notify_l3_agent_new_port(
+                'port', 'after_create', mock.ANY, **kwargs)
+            l3plugin.dvr_vmarp_table_update.assert_called_once_with(
+                self.adminContext, kwargs.get('port'), 'add')
+            l3plugin.dvr_update_router_addvm.assert_called_once_with(
+                self.adminContext, kwargs.get('port'))
+
+    def test__notify_l3_agent_new_port_no_action(self):
+        kwargs = {
+            'context': self.adminContext,
+            'original_port': None,
+            'port': {
+                'device_owner': 'network:None',
+            }
+        }
+        l3plugin = mock.Mock()
+        with mock.patch.object(manager.NeutronManager,
+                               'get_service_plugins',
+                               return_value={'L3_ROUTER_NAT': l3plugin}):
+            l3_dvrscheduler_db._notify_l3_agent_new_port(
+                'port', 'after_create', mock.ANY, **kwargs)
+            self.assertFalse(l3plugin.dvr_vmarp_table_update.called)
+            self.assertFalse(l3plugin.dvr_update_router_addvm.called)
+
+    def test__notify_l3_agent_update_port_no_action(self):
+        kwargs = {
+            'context': self.adminContext,
+            'original_port': {
+                portbindings.HOST_ID: 'vm-host',
+                'device_owner': DEVICE_OWNER_COMPUTE,
+            },
+            'port': {
+                portbindings.HOST_ID: 'vm-host',
+                'device_owner': DEVICE_OWNER_COMPUTE,
+            },
+        }
+        l3plugin = mock.Mock()
+        with mock.patch.object(manager.NeutronManager,
+                               'get_service_plugins',
+                               return_value={'L3_ROUTER_NAT': l3plugin}):
+            l3_dvrscheduler_db._notify_l3_agent_port_update(
+                'port', 'after_update', mock.ANY, **kwargs)
 
             self.assertFalse(l3plugin.dvr_vmarp_table_update.called)
             self.assertFalse(l3plugin.dvr_update_router_addvm.called)
             self.assertFalse(l3plugin.remove_router_from_l3_agent.called)
             self.assertFalse(l3plugin.dvr_deletens_if_no_port.called)
 
+    def test__notify_l3_agent_update_port_with_mac_address_update(self):
+        kwargs = {
+            'context': self.adminContext,
+            'original_port': {
+                portbindings.HOST_ID: 'vm-host',
+                'mac_address': '02:04:05:17:18:19'
+            },
+            'port': {
+                portbindings.HOST_ID: 'vm-host',
+                'mac_address': '02:04:05:17:18:29'
+            },
+            'mac_address_updated': True
+        }
+        l3plugin = mock.Mock()
+        with mock.patch.object(manager.NeutronManager,
+                               'get_service_plugins',
+                               return_value={'L3_ROUTER_NAT': l3plugin}):
+            l3_dvrscheduler_db._notify_l3_agent_port_update(
+                'port', 'after_update', mock.ANY, **kwargs)
+
+            l3plugin.dvr_vmarp_table_update.assert_called_once_with(
+                self.adminContext, kwargs.get('port'), 'add')
+            self.assertFalse(l3plugin.dvr_update_router_addvm.called)
+
+    def test__notify_l3_agent_update_port_with_port_binding_change(self):
+        kwargs = {
+            'context': self.adminContext,
+            'original_port': {
+                'id': str(uuid.uuid4()),
+                portbindings.HOST_ID: 'vm-host1',
+                'device_owner': DEVICE_OWNER_COMPUTE,
+            },
+            'port': {
+                portbindings.HOST_ID: 'vm-host2',
+                'device_owner': DEVICE_OWNER_COMPUTE,
+            },
+        }
+        l3plugin = mock.Mock()
+        with mock.patch.object(manager.NeutronManager,
+                               'get_service_plugins',
+                               return_value={'L3_ROUTER_NAT': l3plugin}),\
+                mock.patch.object(l3plugin, 'dvr_deletens_if_no_port',
+                                  return_value=[{'agent_id': 'foo_agent',
+                                                 'router_id': 'foo_id'}]):
+            l3_dvrscheduler_db._notify_l3_agent_port_update(
+                'port', 'after_update', mock.ANY, **kwargs)
+            l3plugin.remove_router_from_l3_agent.assert_called_once_with(
+                self.adminContext, 'foo_agent', 'foo_id')
+            self.assertEqual(2, l3plugin.dvr_vmarp_table_update.call_count)
+            l3plugin.dvr_update_router_addvm.assert_called_once_with(
+                self.adminContext, kwargs.get('port'))
+
     def test__notify_l3_agent_update_port_removing_routers(self):
         port_id = 'fake-port'
         kwargs = {