From: Oleg Bondarev Date: Thu, 14 May 2015 12:09:24 +0000 (+0300) Subject: Cleanup stale metadata processes on l3 agent sync X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=b058658780f0ccb2787c26e3e95cabcc8e2e6349;p=openstack-build%2Fneutron-build.git Cleanup stale metadata processes on l3 agent sync Currently l3 agent only cleans up stale namespaces. The fix adds checking and deleting stale metadata processes to NamespaceManager class responsible for clearing stale namespaces Change-Id: I2b081803e312589d3d8a7808d286a6c9827ef53f Closes-Bug: #1455042 --- diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index 01395d34c..9959fddc1 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -208,10 +208,15 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, continue break + self.metadata_driver = None + if self.conf.enable_metadata_proxy: + self.metadata_driver = metadata_driver.MetadataDriver(self) + self.namespaces_manager = namespace_manager.NamespaceManager( self.conf, self.driver, - self.conf.use_namespaces) + self.conf.use_namespaces, + self.metadata_driver) self._queue = queue.RouterProcessingQueue() super(L3NATAgent, self).__init__(conf=self.conf) @@ -219,9 +224,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, self.target_ex_net_id = None self.use_ipv6 = ipv6_utils.is_enabled() - if self.conf.enable_metadata_proxy: - self.metadata_driver = metadata_driver.MetadataDriver(self) - def _check_config_params(self): """Check items in configuration files. diff --git a/neutron/agent/l3/namespace_manager.py b/neutron/agent/l3/namespace_manager.py index 2f35b86ba..e7d029fcd 100644 --- a/neutron/agent/l3/namespace_manager.py +++ b/neutron/agent/l3/namespace_manager.py @@ -14,6 +14,7 @@ from oslo_log import log as logging from neutron.agent.l3 import dvr_snat_ns from neutron.agent.l3 import namespaces +from neutron.agent.linux import external_process from neutron.agent.linux import ip_lib from neutron.i18n import _LE @@ -41,16 +42,22 @@ class NamespaceManager(object): agent restarts gracefully. """ - def __init__(self, agent_conf, driver, clean_stale): + def __init__(self, agent_conf, driver, clean_stale, metadata_driver=None): """Initialize the NamespaceManager. :param agent_conf: configuration from l3 agent :param driver: to perform operations on devices :param clean_stale: Whether to try to clean stale namespaces + :param metadata_driver: used to cleanup stale metadata proxy processes """ self.agent_conf = agent_conf self.driver = driver self._clean_stale = clean_stale + self.metadata_driver = metadata_driver + if metadata_driver: + self.process_monitor = external_process.ProcessMonitor( + config=agent_conf, + resource_type='router') def __enter__(self): self._all_namespaces = set() @@ -85,6 +92,10 @@ class NamespaceManager(object): self.driver, use_ipv6=False) try: + if self.metadata_driver: + # cleanup stale metadata proxy processes first + self.metadata_driver.destroy_monitored_metadata_proxy( + self.process_monitor, ns_id, self.agent_conf) ns.delete() except RuntimeError: LOG.exception(_LE('Failed to destroy stale namespace %s'), ns) diff --git a/neutron/tests/functional/agent/l3/test_namespace_manager.py b/neutron/tests/functional/agent/l3/test_namespace_manager.py index 51922e6ff..69a571858 100755 --- a/neutron/tests/functional/agent/l3/test_namespace_manager.py +++ b/neutron/tests/functional/agent/l3/test_namespace_manager.py @@ -31,8 +31,10 @@ class NamespaceManagerTestFramework(base.BaseSudoTestCase): super(NamespaceManagerTestFramework, self).setUp() self.agent_conf = mock.MagicMock() self.agent_conf.router_delete_namespaces = True + self.metadata_driver_mock = mock.Mock() self.namespace_manager = namespace_manager.NamespaceManager( - self.agent_conf, driver=None, clean_stale=True) + self.agent_conf, driver=None, clean_stale=True, + metadata_driver=self.metadata_driver_mock) def _create_namespace(self, router_id, ns_class): namespace = ns_class(router_id, self.agent_conf, driver=None, @@ -59,6 +61,7 @@ class NamespaceManagerTestCase(NamespaceManagerTestFramework): def test_namespace_manager(self): router_id = _uuid() + router_id_to_delete = _uuid() to_keep = set() to_delete = set() to_retrieve = set() @@ -66,7 +69,7 @@ class NamespaceManagerTestCase(NamespaceManagerTestFramework): namespaces.RouterNamespace)) to_keep.add(self._create_namespace(router_id, dvr_snat_ns.SnatNamespace)) - to_delete.add(self._create_namespace(_uuid(), + to_delete.add(self._create_namespace(router_id_to_delete, dvr_snat_ns.SnatNamespace)) to_retrieve = to_keep | to_delete @@ -80,4 +83,8 @@ class NamespaceManagerTestCase(NamespaceManagerTestFramework): for ns_name in to_keep: self.assertTrue(self._namespace_exists(ns_name)) for ns_name in to_delete: + (self.metadata_driver_mock.destroy_monitored_metadata_proxy. + assert_called_once_with(mock.ANY, + router_id_to_delete, + self.agent_conf)) self.assertFalse(self._namespace_exists(ns_name)) diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py index 98e5d661e..c979c03b6 100644 --- a/neutron/tests/functional/agent/test_l3_agent.py +++ b/neutron/tests/functional/agent/test_l3_agent.py @@ -63,7 +63,8 @@ def get_ovs_bridge(br_name): class L3AgentTestFramework(base.BaseSudoTestCase): def setUp(self): super(L3AgentTestFramework, self).setUp() - mock.patch('neutron.agent.l3.agent.L3PluginApi').start() + self.mock_plugin_api = mock.patch( + 'neutron.agent.l3.agent.L3PluginApi').start().return_value mock.patch('neutron.agent.rpc.PluginReportStateAPI').start() self.agent = self._configure_agent('agent1') @@ -500,23 +501,23 @@ class L3AgentTestCase(L3AgentTestFramework): routers_to_keep = [] routers_to_delete = [] ns_names_to_retrieve = set() + routers_info_to_delete = [] for i in range(2): routers_to_keep.append(self.generate_router_info(False)) - self.manage_router(self.agent, routers_to_keep[i]) - ns_names_to_retrieve.add(namespaces.NS_PREFIX + - routers_to_keep[i]['id']) + ri = self.manage_router(self.agent, routers_to_keep[i]) + ns_names_to_retrieve.add(ri.ns_name) for i in range(2): routers_to_delete.append(self.generate_router_info(False)) - self.manage_router(self.agent, routers_to_delete[i]) - ns_names_to_retrieve.add(namespaces.NS_PREFIX + - routers_to_delete[i]['id']) + ri = self.manage_router(self.agent, routers_to_delete[i]) + routers_info_to_delete.append(ri) + ns_names_to_retrieve.add(ri.ns_name) # Mock the plugin RPC API to Simulate a situation where the agent # was handling the 4 routers created above, it went down and after # starting up again, two of the routers were deleted via the API - mocked_get_routers = ( - neutron_l3_agent.L3PluginApi.return_value.get_routers) - mocked_get_routers.return_value = routers_to_keep + self.mock_plugin_api.get_routers.return_value = routers_to_keep + # also clear agent router_info as it will be after restart + self.agent.router_info = {} # Synchonize the agent with the plug-in with mock.patch.object(namespace_manager.NamespaceManager, 'list_all', @@ -526,9 +527,8 @@ class L3AgentTestCase(L3AgentTestFramework): # Mock the plugin RPC API so a known external network id is returned # when the router updates are processed by the agent external_network_id = _uuid() - mocked_get_external_network_id = ( - neutron_l3_agent.L3PluginApi.return_value.get_external_network_id) - mocked_get_external_network_id.return_value = external_network_id + self.mock_plugin_api.get_external_network_id.return_value = ( + external_network_id) # Plug external_gateway_info in the routers that are not going to be # deleted by the agent when it processes the updates. Otherwise, @@ -539,7 +539,7 @@ class L3AgentTestCase(L3AgentTestFramework): # Have the agent process the update from the plug-in and verify # expected behavior - for _ in routers_to_keep + routers_to_delete: + for _ in routers_to_keep: self.agent._process_router_update() for i in range(2): @@ -547,10 +547,9 @@ class L3AgentTestCase(L3AgentTestFramework): self.assertTrue(self._namespace_exists(namespaces.NS_PREFIX + routers_to_keep[i]['id'])) for i in range(2): - self.assertNotIn(routers_to_delete[i]['id'], + self.assertNotIn(routers_info_to_delete[i].router_id, self.agent.router_info) - self.assertFalse(self._namespace_exists( - namespaces.NS_PREFIX + routers_to_delete[i]['id'])) + self._assert_router_does_not_exist(routers_info_to_delete[i]) def _router_lifecycle(self, enable_ha, ip_version=4, dual_stack=False, v6_ext_gw_with_sub=True): @@ -948,9 +947,7 @@ class TestDvrRouter(L3AgentTestFramework): self, agent_mode, **dvr_router_kwargs): self.agent.conf.agent_mode = agent_mode router_info = self.generate_dvr_router_info(**dvr_router_kwargs) - mocked_ext_net_id = ( - neutron_l3_agent.L3PluginApi.return_value.get_external_network_id) - mocked_ext_net_id.return_value = ( + self.mock_plugin_api.get_external_network_id.return_value = ( router_info['_floatingips'][0]['floating_network_id']) router = self.manage_router(self.agent, router_info) fip_ns = router.fip_ns.get_name() @@ -1010,15 +1007,12 @@ class TestDvrRouter(L3AgentTestFramework): # gateway_port information before the l3_agent will create it. # The port returned needs to have the same information as # router_info['gw_port'] - mocked_gw_port = ( - neutron_l3_agent.L3PluginApi.return_value.get_agent_gateway_port) - mocked_gw_port.return_value = router_info['gw_port'] + self.mock_plugin_api.get_agent_gateway_port.return_value = router_info[ + 'gw_port'] # We also need to mock the get_external_network_id method to # get the correct fip namespace. - mocked_ext_net_id = ( - neutron_l3_agent.L3PluginApi.return_value.get_external_network_id) - mocked_ext_net_id.return_value = ( + self.mock_plugin_api.get_external_network_id.return_value = ( router_info['_floatingips'][0]['floating_network_id']) # With all that set we can now ask the l3_agent to diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index e628cd465..885e21bf1 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -426,6 +426,26 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): agent.periodic_sync_routers_task(agent.context) self.assertFalse(agent.namespaces_manager._clean_stale) + def test_periodic_sync_routers_task_call_clean_stale_meta_proxies(self): + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + stale_router_ids = [_uuid(), _uuid()] + active_routers = [{'id': _uuid()}, {'id': _uuid()}] + self.plugin_api.get_routers.return_value = active_routers + namespace_list = [namespaces.NS_PREFIX + r_id + for r_id in stale_router_ids] + namespace_list += [namespaces.NS_PREFIX + r['id'] + for r in active_routers] + self.mock_ip.get_namespaces.return_value = namespace_list + driver = metadata_driver.MetadataDriver + with mock.patch.object( + driver, 'destroy_monitored_metadata_proxy') as destroy_proxy: + agent.periodic_sync_routers_task(agent.context) + + expected_calls = [mock.call(mock.ANY, r_id, agent.conf) + for r_id in stale_router_ids] + self.assertEqual(len(stale_router_ids), destroy_proxy.call_count) + destroy_proxy.assert_has_calls(expected_calls, any_order=True) + def test_router_info_create(self): id = _uuid() ri = l3router.RouterInfo(id, {}, **self.ri_kwargs)