openstack
/
neutron
Code Issues Proposed changes

ProcessManager: honor run_as_root when stopping process 

Without this commit, the run_as_root parameter is always True when
stopping a process, which leads to the usage of unnecessary sudo such as
in some functional tests, like the keepalived ones.

This commit fixes the aforemetioned problem by taking run_as_root into
account when stopping a process. However, run_as_root will still always
be True if the process is spawned in a netns.

Closes-Bug: #1491581

Change-Id: Ib40e1e3357b9a38e760f4e552bf615cdfd54ee5a
Signed-off-by: Hunt Xu <mhuntxu@gmail.com>
This commit is contained in:
Hunt Xu 2017-04-22 01:08:48 +08:00
parent 120cce55db
commit a15c849563
11 changed files with 32 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
neutron/agent/dhcp/agent.py
View File

@ -523,7 +523,7 @@ class DhcpAgent(manager.Manager):
uuid = network.id
is_router_id = False
metadata_driver.MetadataDriver.destroy_monitored_metadata_proxy(
self._process_monitor, uuid, self.conf)
self._process_monitor, uuid, self.conf, network.namespace)
if is_router_id:
del self._metadata_routers[network.id]

2
neutron/agent/l3/ha.py
View File

@ -161,7 +161,7 @@ class AgentMixin(object):
else:
LOG.debug('Closing metadata proxy for router %s', router_id)
self.metadata_driver.destroy_monitored_metadata_proxy(
self.process_monitor, ri.router_id, self.conf)
self.process_monitor, ri.router_id, self.conf, ri.ns_name)
def _update_radvd_daemon(self, ri, state):
# Radvd has to be spawned only on the Master HA Router. If there are

2
neutron/agent/l3/namespace_manager.py
View File

@ -141,7 +141,7 @@ class NamespaceManager(object):
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)
self.process_monitor, ns_id, self.agent_conf, ns.name)
ns.delete()
except RuntimeError:
LOG.exception(_LE('Failed to destroy stale namespace %s'), ns)

7
neutron/agent/linux/external_process.py
View File

@ -72,7 +72,7 @@ class ProcessManager(MonitoredProcess):
self.cmd_addl_env = cmd_addl_env
self.pids_path = pids_path or self.conf.external_pids
self.pid_file = pid_file
self.run_as_root = run_as_root
self.run_as_root = run_as_root or self.namespace is not None
self.custom_reload_callback = custom_reload_callback
if service:
@ -110,10 +110,11 @@ class ProcessManager(MonitoredProcess):
if get_stop_command:
cmd = get_stop_command(self.get_pid_file_name())
ip_wrapper = ip_lib.IPWrapper(namespace=self.namespace)
ip_wrapper.netns.execute(cmd, addl_env=self.cmd_addl_env)
ip_wrapper.netns.execute(cmd, addl_env=self.cmd_addl_env,
run_as_root=self.run_as_root)
else:
cmd = ['kill', '-%s' % (sig), pid]
utils.execute(cmd, run_as_root=True)
utils.execute(cmd, run_as_root=self.run_as_root)
# In the case of shutting down, remove the pid file
if sig == '9':
fileutils.delete_if_exists(self.get_pid_file_name())

9
neutron/agent/metadata/driver.py
View File

@ -259,10 +259,10 @@ class MetadataDriver(object):
pm.disable()
@classmethod
def destroy_monitored_metadata_proxy(cls, monitor, uuid, conf):
def destroy_monitored_metadata_proxy(cls, monitor, uuid, conf, ns_name):
monitor.unregister(uuid, METADATA_SERVICE_NAME)
# No need to pass ns name as it's not needed for disable()
pm = cls._get_metadata_proxy_process_manager(uuid, conf)
pm = cls._get_metadata_proxy_process_manager(uuid, conf,
ns_name=ns_name)
pm.disable()
# Delete metadata proxy config file
@ -320,4 +320,5 @@ def before_router_removed(resource, event, l3_agent, **kwargs):
proxy.destroy_monitored_metadata_proxy(l3_agent.process_monitor,
router.router['id'],
l3_agent.conf)
l3_agent.conf,
router.ns_name)

3
neutron/tests/functional/agent/l3/test_namespace_manager.py
View File

@ -85,5 +85,6 @@ class NamespaceManagerTestCase(NamespaceManagerTestFramework):
(self.metadata_driver_mock.destroy_monitored_metadata_proxy.
assert_called_once_with(mock.ANY,
router_id_to_delete,
self.agent_conf))
self.agent_conf,
ns_name))
self.assertFalse(self._namespace_exists(ns_name))

2
neutron/tests/functional/agent/linux/test_keepalived.py
View File

@ -63,7 +63,7 @@ class KeepalivedManagerTestCase(base.BaseLoggingTestCase,
# Exit the process, and see that when it comes back
# It's indeed a different process
utils.execute(['kill', exit_code, pid], run_as_root=True)
utils.execute(['kill', exit_code, pid])
common_utils.wait_until_true(
lambda: process.active and pid != process.pid,
timeout=5,

14
neutron/tests/unit/agent/dhcp/test_agent.py
View File

@ -482,7 +482,7 @@ class TestDhcpAgent(base.BaseTestCase):
network_id=fake_network.id)
dhcp.disable_dhcp_helper(fake_network.id)
md_cls.destroy_monitored_metadata_proxy.assert_called_once_with(
mock.ANY, fake_network.id, mock.ANY)
mock.ANY, fake_network.id, mock.ANY, fake_network.namespace)
def test_report_state_revival_logic(self):
dhcp = dhcp_agent.DhcpAgentWithStateReport(HOSTNAME)
@ -678,7 +678,7 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
mock.call().enable()], any_order=True)
else:
self.external_process.assert_has_calls([
self._process_manager_constructor_call(ns=None),
self._process_manager_constructor_call(),
mock.call().disable()])
def test_enable_dhcp_helper_enable_metadata_isolated_network(self):
@ -796,7 +796,7 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
[mock.call.get_network_by_id(fake_network.id)])
self.call_driver.assert_called_once_with('disable', fake_network)
self.external_process.assert_has_calls([
self._process_manager_constructor_call(ns=None),
self._process_manager_constructor_call(),
mock.call().disable()])
def test_disable_dhcp_helper_known_network_isolated_metadata(self):
@ -825,7 +825,7 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
self.cache.assert_has_calls(
[mock.call.get_network_by_id(fake_network.id)])
self.external_process.assert_has_calls([
self._process_manager_constructor_call(ns=None),
self._process_manager_constructor_call(),
mock.call().disable()
])
@ -850,7 +850,8 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
self.dhcp.disable_isolated_metadata_proxy(fake_network)
destroy.assert_called_once_with(self.dhcp._process_monitor,
fake_network.id,
cfg.CONF)
cfg.CONF,
fake_network.namespace)
def _test_enable_isolated_metadata_proxy(self, network):
cfg.CONF.set_override('enable_metadata_network', True)
@ -884,7 +885,8 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
self.dhcp.disable_isolated_metadata_proxy(network)
destroy.assert_called_once_with(self.dhcp._process_monitor,
'forzanapoli',
cfg.CONF)
cfg.CONF,
network.namespace)
def test_disable_isolated_metadata_proxy_with_metadata_network(self):
self._test_disable_isolated_metadata_proxy(fake_meta_network)

9
neutron/tests/unit/agent/l3/test_agent.py
View File

@ -360,8 +360,10 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
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]
expected_calls = [
mock.call(
mock.ANY, r_id, agent.conf, namespaces.NS_PREFIX + r_id)
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)
@ -2168,7 +2170,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
if enableflag:
destroy_proxy.assert_called_with(mock.ANY,
router_id,
mock.ANY)
mock.ANY,
'qrouter-' + router_id)
else:
self.assertFalse(destroy_proxy.call_count)

4
neutron/tests/unit/agent/linux/test_external_process.py
View File

@ -154,7 +154,7 @@ class TestProcessManager(base.BaseTestCase):
ip_lib.assert_has_calls([
mock.call.IPWrapper(namespace='ns'),
mock.call.IPWrapper().netns.execute(
['the', 'cmd'], addl_env=None, run_as_root=False)])
['the', 'cmd'], addl_env=None, run_as_root=True)])
def test_enable_with_namespace_process_active(self):
callback = mock.Mock()
@ -208,7 +208,7 @@ class TestProcessManager(base.BaseTestCase):
manager.disable()
utils.assert_has_calls([
mock.call.execute(['kill', '-9', 4],
run_as_root=True)])
run_as_root=False)])
def test_disable_namespace(self):
with mock.patch.object(ep.ProcessManager, 'pid') as pid:

2
neutron/tests/unit/agent/metadata/test_driver.py
View File

@ -172,7 +172,7 @@ class TestMetadataDriverProcess(base.BaseTestCase):
ip_mock.assert_has_calls([
mock.call(namespace=router_ns),
mock.call().netns.execute(netns_execute_args, addl_env=None,
run_as_root=False)
run_as_root=True)
])
def test_create_config_file_wrong_user(self):