From b223452e68fa8629cb67826bb8ae3ac612e0b859 Mon Sep 17 00:00:00 2001 From: Swaminathan Vasudevan Date: Fri, 4 Dec 2015 11:58:57 -0800 Subject: [PATCH] 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 --- neutron/db/l3_dvrscheduler_db.py | 29 +++-- .../unit/scheduler/test_l3_agent_scheduler.py | 111 ++++++++++++++++++ 2 files changed, 128 insertions(+), 12 deletions(-) diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py index a70b0e49525..449270cef65 100644 --- 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 32730f457a4..0f6e9a56446 100644 --- 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 = {