From: Stephen Ma Date: Fri, 28 Aug 2015 14:00:48 +0000 (+0000) Subject: Descheduling DVR routers when ports are unbound from VM X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=d5a8074ec3c67ed68e64a96827da990f1c34e10f;p=openstack-build%2Fneutron-build.git Descheduling DVR routers when ports are unbound from VM When a VM is deleted, the DVR port used by the VM could be unbound from the compute node. When it is unbounded, it is no longer in use on the node. Currently the unbind doesn't trigger a check to determine whether the DVR router can be unscheduled from the L3-agent running on the compute node. This patch makes the check and unschedule the router, if necessary. Closes-Bug: 1489184 Change-Id: I882e0682bfc7695b3b23e36eb4d7e35a5d19748e --- diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py index 6caba1494..59f3f3fa1 100644 --- a/neutron/db/l3_dvrscheduler_db.py +++ b/neutron/db/l3_dvrscheduler_db.py @@ -161,12 +161,17 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin): return True return False - def dvr_deletens_if_no_port(self, context, port_id): + def dvr_deletens_if_no_port(self, context, port_id, port_host=None): """Delete the DVR namespace if no dvr serviced port exists.""" admin_context = context.elevated() router_ids = self.get_dvr_routers_by_portid(admin_context, port_id) - port_host = ml2_db.get_port_binding_host(admin_context.session, - port_id) + if not port_host: + port_host = ml2_db.get_port_binding_host(admin_context.session, + port_id) + if not port_host: + LOG.debug('Host name not found for port %s', port_id) + return [] + if not router_ids: LOG.debug('No namespaces available for this DVR port %(port)s ' 'on host %(host)s', {'port': port_id, @@ -508,9 +513,37 @@ def _notify_port_delete(event, resource, trigger, **kwargs): context, router['agent_id'], router['router_id']) +def _notify_l3_agent_port_update(resource, event, trigger, **kwargs): + new_port = kwargs.get('port') + original_port = kwargs.get('original_port') + + if new_port and original_port: + original_device_owner = original_port.get('device_owner', '') + if (original_device_owner.startswith('compute') and + not new_port.get('device_owner')): + 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'], + port_host=original_port['binding:host_id']) + if removed_routers: + removed_router_args = { + 'context': context, + 'port': original_port, + 'removed_routers': removed_routers, + } + _notify_port_delete( + event, resource, trigger, **removed_router_args) + return + + _notify_l3_agent_new_port(resource, event, trigger, **kwargs) + + def subscribe(): registry.subscribe( - _notify_l3_agent_new_port, resources.PORT, events.AFTER_UPDATE) + _notify_l3_agent_port_update, resources.PORT, events.AFTER_UPDATE) registry.subscribe( _notify_l3_agent_new_port, resources.PORT, events.AFTER_CREATE) registry.subscribe( diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 2b26e735f..72658d18b 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -194,9 +194,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, def _get_host_port_if_changed(self, mech_context, attrs): binding = mech_context._binding - host = attrs and attrs.get(portbindings.HOST_ID) - if (attributes.is_attr_set(host) and binding.host != host): - return mech_context.current + if attrs and portbindings.HOST_ID in attrs: + if binding.host != attrs.get(portbindings.HOST_ID): + return mech_context.current def _check_mac_update_allowed(self, orig_port, port, binding): unplugged_types = (portbindings.VIF_TYPE_BINDING_FAILED, @@ -1208,6 +1208,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, 'context': context, 'port': new_host_port, 'mac_address_updated': mac_address_updated, + 'original_port': original_port, } registry.notify(resources.PORT, events.AFTER_UPDATE, self, **kwargs) diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index 7dc84b50c..0334ad8f4 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -1617,6 +1617,64 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase): return plugin + def _test__get_host_port_if_changed( + self, mech_context, attrs=None, expected_retval=None): + with mock.patch.object(ml2_plugin.Ml2Plugin, + '__init__', + return_value=None): + plugin = ml2_plugin.Ml2Plugin() + test_return = plugin._get_host_port_if_changed(mech_context, attrs) + self.assertEqual(expected_retval, test_return) + + def test__get_host_port_if_changed_no_attrs(self): + mech_context = mock.Mock() + mech_context._binding.host = 'Host-1' + self._test__get_host_port_if_changed( + mech_context, attrs=None, expected_retval=None) + + def test__get_host_port_if_changed_no_binding_change(self): + mech_context = mock.Mock() + mech_context._binding.host = 'Host-1' + mech_context.current = { + 'id': 'fake-id', + 'mac_address': '2a:2b:2c:2d:2e:2f' + } + attrs = {'mac_address': '0a:0b:0c:0d:0e:0f'} + self._test__get_host_port_if_changed( + mech_context, attrs=attrs, expected_retval=None) + + attrs = { + portbindings.HOST_ID: 'Host-1', + 'mac_address': '0a:0b:0c:0d:0e:0f', + } + self._test__get_host_port_if_changed( + mech_context, attrs=attrs, expected_retval=None) + + def test__get_host_port_if_changed_with_binding_removed(self): + expected_return = { + 'id': 'fake-id', + portbindings.HOST_ID: None, + 'mac_address': '2a:2b:2c:2d:2e:2f' + } + mech_context = mock.Mock() + mech_context._binding.host = 'Host-1' + mech_context.current = expected_return + attrs = {portbindings.HOST_ID: None} + self._test__get_host_port_if_changed( + mech_context, attrs=attrs, expected_retval=expected_return) + + def test__get_host_port_if_changed_with_binding_added(self): + expected_return = { + 'id': 'fake-id', + portbindings.HOST_ID: 'host-1', + 'mac_address': '2a:2b:2c:2d:2e:2f' + } + mech_context = mock.Mock() + mech_context.current = expected_return + attrs = {portbindings.HOST_ID: 'host-1'} + self._test__get_host_port_if_changed( + mech_context, attrs=attrs, expected_retval=expected_return) + def test_create_port_rpc_outside_transaction(self): with mock.patch.object(ml2_plugin.Ml2Plugin, '__init__') as init,\ mock.patch.object(base_plugin.NeutronDbPluginV2, @@ -1633,19 +1691,43 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase): plugin, **kwargs) def test_update_port_rpc_outside_transaction(self): + port_id = 'fake_id' + net_id = 'mynet' + original_port_db = models_v2.Port( + id=port_id, + tenant_id='tenant', + network_id=net_id, + mac_address='08:00:01:02:03:04', + admin_state_up=True, + status='ACTIVE', + device_id='vm_id', + device_owner='compute:None') + + binding = mock.Mock() + binding.port_id = port_id + binding.host = 'vm_host' + binding.vnic_type = portbindings.VNIC_NORMAL + binding.profile = '' + binding.vif_type = '' + binding.vif_details = '' + with mock.patch.object(ml2_plugin.Ml2Plugin, '__init__') as init,\ + mock.patch.object(ml2_db, 'get_locked_port_and_binding', + return_value=(original_port_db, binding)),\ mock.patch.object(base_plugin.NeutronDbPluginV2, 'update_port'): init.return_value = None new_host_port = mock.Mock() plugin = self._create_plugin_for_create_update_port(new_host_port) + original_port = plugin._make_port_dict(original_port_db) - plugin.update_port(self.context, 'fake_id', mock.MagicMock()) + plugin.update_port(self.context, port_id, mock.MagicMock()) kwargs = { 'context': self.context, 'port': new_host_port, 'mac_address_updated': True, + 'original_port': original_port, } self.notify.assert_called_once_with('port', 'after_update', plugin, **kwargs) diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py index e4421e4aa..30c5a97db 100644 --- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py +++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py @@ -913,6 +913,81 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase): self.adminContext = n_context.get_admin_context() self.dut = L3DvrScheduler() + def test__notify_l3_agent_update_port_no_removing_routers(self): + port_id = 'fake-port' + kwargs = { + 'context': self.adminContext, + 'port': None, + 'original_port': { + 'id': port_id, + 'binding:host_id': 'vm-host', + 'device_id': 'vm-id', + 'device_owner': 'compute:None', + 'mac_address': '02:04:05:17:18:19' + }, + 'mac_address_updated': True + } + + plugin = manager.NeutronManager.get_plugin() + l3plugin = mock.Mock() + l3plugin.supported_extension_aliases = [ + 'router', constants.L3_AGENT_SCHEDULER_EXT_ALIAS, + constants.L3_DISTRIBUTED_EXT_ALIAS + ] + + 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', 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_update_port_removing_routers(self): + port_id = 'fake-port' + kwargs = { + 'context': self.adminContext, + 'port': { + 'id': port_id, + 'binding:host_id': None, + 'device_id': '', + 'device_owner': '' + }, + 'mac_address_updated': False, + 'original_port': { + 'id': port_id, + 'binding:host_id': 'vm-host', + 'device_id': 'vm-id', + 'device_owner': 'compute:None' + } + } + + plugin = manager.NeutronManager.get_plugin() + l3plugin = mock.Mock() + l3plugin.supported_extension_aliases = [ + 'router', constants.L3_AGENT_SCHEDULER_EXT_ALIAS, + constants.L3_DISTRIBUTED_EXT_ALIAS + ] + 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', plugin, **kwargs) + + self.assertEqual(1, l3plugin.dvr_vmarp_table_update.call_count) + l3plugin.dvr_vmarp_table_update.assert_called_once_with( + self.adminContext, mock.ANY, 'del') + + self.assertFalse(l3plugin.dvr_update_router_addvm.called) + l3plugin.remove_router_from_l3_agent.assert_called_once_with( + self.adminContext, 'foo_agent', 'foo_id') + def test__notify_port_delete(self): plugin = manager.NeutronManager.get_plugin() l3plugin = mock.Mock()