From 31f9ce541c11464a330d6e850e2848ae47a85363 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 10 Dec 2020 16:59:46 +0000 Subject: [PATCH] Delete HA metadata proxy PID and config with elevated privileges Both files cannot be deleted with the default permissions because those files are created by the "root" user. Change-Id: I73dd37b3104fac8d3172f520f71cffd85d040c4b Closes-Bug: #1907695 (cherry picked from commit 0a0f647ea05d3de3820e9dab26ded8c528eee835) --- neutron/agent/linux/external_process.py | 3 ++- neutron/agent/linux/utils.py | 9 +++++++ neutron/agent/metadata/driver.py | 10 ++------ neutron/privileged/agent/linux/utils.py | 7 ++++++ .../tests/functional/agent/l3/framework.py | 24 +++++++++++++++---- neutron/tests/unit/agent/dhcp/test_agent.py | 2 ++ neutron/tests/unit/agent/l3/test_agent.py | 15 +++++++----- 7 files changed, 50 insertions(+), 20 deletions(-) diff --git a/neutron/agent/linux/external_process.py b/neutron/agent/linux/external_process.py index 3a25a946ffd..ae7a42bf41d 100644 --- a/neutron/agent/linux/external_process.py +++ b/neutron/agent/linux/external_process.py @@ -111,7 +111,8 @@ class ProcessManager(MonitoredProcess): 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()) + utils.delete_if_exists(self.get_pid_file_name(), + run_as_root=self.run_as_root) elif pid: LOG.debug('%(service)s process for %(uuid)s pid %(pid)d is stale, ' 'ignoring signal %(signal)s', diff --git a/neutron/agent/linux/utils.py b/neutron/agent/linux/utils.py index 3cf60127d24..a894845f423 100644 --- a/neutron/agent/linux/utils.py +++ b/neutron/agent/linux/utils.py @@ -38,6 +38,7 @@ from neutron._i18n import _ from neutron.agent.linux import xenapi_root_helper from neutron.common import utils from neutron.conf.agent import common as config +from neutron.privileged.agent.linux import utils as priv_utils from neutron import wsgi @@ -403,6 +404,14 @@ def is_effective_group(group_id_or_name): return group_id_or_name == effective_group_name +def delete_if_exists(path, run_as_root=False): + """Delete a path, executed as normal user or with elevated privileges""" + if run_as_root: + priv_utils.delete_if_exists(path) + else: + fileutils.delete_if_exists(path) + + class UnixDomainHTTPConnection(httplib.HTTPConnection): """Connection class for HTTP over UNIX domain socket.""" def __init__(self, host, port=None, strict=None, timeout=None, diff --git a/neutron/agent/metadata/driver.py b/neutron/agent/metadata/driver.py index 191d920ad86..cac59b499ec 100644 --- a/neutron/agent/metadata/driver.py +++ b/neutron/agent/metadata/driver.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -import errno import grp import os import pwd @@ -32,6 +31,7 @@ from neutron.agent.l3 import ha_router from neutron.agent.l3 import namespaces from neutron.agent.linux import external_process from neutron.agent.linux import ip_lib +from neutron.agent.linux import utils as linux_utils LOG = logging.getLogger(__name__) @@ -175,13 +175,7 @@ class HaproxyConfigurator(object): cfg_path = os.path.join( HaproxyConfigurator.get_config_path(state_path), "%s.conf" % uuid) - try: - os.unlink(cfg_path) - except OSError as ex: - # It can happen that this function is called but metadata proxy - # was never spawned so its config file won't exist - if ex.errno != errno.ENOENT: - raise + linux_utils.delete_if_exists(cfg_path, run_as_root=True) class MetadataDriver(object): diff --git a/neutron/privileged/agent/linux/utils.py b/neutron/privileged/agent/linux/utils.py index c038814acea..edceb643d61 100644 --- a/neutron/privileged/agent/linux/utils.py +++ b/neutron/privileged/agent/linux/utils.py @@ -12,9 +12,11 @@ # License for the specific language governing permissions and limitations # under the License. +import os import re from oslo_concurrency import processutils +from oslo_utils import fileutils from neutron import privileged @@ -40,3 +42,8 @@ def _find_listen_pids_namespace(namespace): if m: pids.add(m.group('pid')) return list(pids) + + +@privileged.default.entrypoint +def delete_if_exists(path, remove=os.unlink): + fileutils.delete_if_exists(path, remove=remove) diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index 4b4edb49b0b..edacfb5157b 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -15,6 +15,7 @@ import copy import functools +import os from unittest import mock import netaddr @@ -435,12 +436,15 @@ class L3AgentTestFramework(base.BaseSudoTestCase): def _namespace_exists(self, namespace): return ip_lib.network_namespace_exists(namespace) - def _metadata_proxy_exists(self, conf, router): - pm = external_process.ProcessManager( + def _metadata_proxy(self, conf, router): + return external_process.ProcessManager( conf, router.router_id, router.ns_name, service=metadata_driver.HAPROXY_SERVICE) + + def _metadata_proxy_exists(self, conf, router): + pm = self._metadata_proxy(conf, router) return pm.active def device_exists_with_ips_and_mac(self, expected_device, name_getter, @@ -501,9 +505,19 @@ class L3AgentTestFramework(base.BaseSudoTestCase): # then the devices and iptable rules have also been deleted, # so there's no need to check that explicitly. self.assertFalse(self._namespace_exists(router.ns_name)) - common_utils.wait_until_true( - lambda: not self._metadata_proxy_exists(self.agent.conf, router), - timeout=10) + try: + common_utils.wait_until_true( + lambda: not self._metadata_proxy_exists(self.agent.conf, + router), + timeout=10) + except common_utils.WaitTimeout: + pm = self._metadata_proxy(self.agent.conf, router) + pid_file = pm.get_pid_file_name() + if os.path.exists(pid_file): + msg = 'PID file %s still exists and it should not.' % pid_file + else: + msg = 'PID file %s is not present.' % pid_file + self.fail(msg) def _assert_snat_chains(self, router): self.assertFalse(router.iptables_manager.is_chain_empty( diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py index a769980ba38..9cdcf83d6a4 100644 --- a/neutron/tests/unit/agent/dhcp/test_agent.py +++ b/neutron/tests/unit/agent/dhcp/test_agent.py @@ -34,6 +34,7 @@ from neutron.agent.dhcp import agent as dhcp_agent from neutron.agent import dhcp_agent as entry from neutron.agent.linux import dhcp from neutron.agent.linux import interface +from neutron.agent.linux import utils as linux_utils from neutron.agent.metadata import driver as metadata_driver from neutron.common import config as common_config from neutron.common import utils @@ -790,6 +791,7 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): 'neutron.agent.linux.ip_lib.' 'IpAddrCommand.wait_until_address_ready') self.mock_wait_until_address_ready_p.start() + mock.patch.object(linux_utils, 'delete_if_exists').start() self.addCleanup(self.mock_wait_until_address_ready_p.stop) def _process_manager_constructor_call(self, ns=FAKE_NETWORK_DHCP_NS): diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 339d15d76ca..9448a73b047 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -54,6 +54,7 @@ from neutron.agent.linux import ip_lib from neutron.agent.linux import iptables_manager from neutron.agent.linux import pd from neutron.agent.linux import ra +from neutron.agent.linux import utils as linux_utils from neutron.agent.metadata import driver as metadata_driver from neutron.agent import rpc as agent_rpc from neutron.conf.agent import common as agent_config @@ -3065,9 +3066,10 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): dvr_snat_ns.SNAT_NS_PREFIX + 'foo'] other_namespaces = ['unknown'] - self._cleanup_namespace_test(stale_namespaces, - [], - other_namespaces) + with mock.patch.object(linux_utils, 'delete_if_exists'): + self._cleanup_namespace_test(stale_namespaces, + [], + other_namespaces) def test_cleanup_namespace_with_registered_router_ids(self): stale_namespaces = [namespaces.NS_PREFIX + 'cccc', @@ -3077,9 +3079,10 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): {'id': 'aaaa', 'distributed': False}] other_namespaces = ['qdhcp-aabbcc', 'unknown'] - self._cleanup_namespace_test(stale_namespaces, - router_list, - other_namespaces) + with mock.patch.object(linux_utils, 'delete_if_exists'): + self._cleanup_namespace_test(stale_namespaces, + router_list, + other_namespaces) def test_create_dvr_gateway(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)