From dad0969a2eed6ee428521784d4d51e3b971c25b4 Mon Sep 17 00:00:00 2001 From: Bogdan Tabor Date: Wed, 7 Oct 2015 15:49:08 +0200 Subject: [PATCH] Switch to using neutron.common.utils:replace_file() neutron.agent.linux.utils:replace_file() and neutron.common.utils:replace_file() have same functionality. This is the 1st patch in the series of 4 patches. It modifies neutron.common.utils:replace_file(), so it can be used by all components as a replacement for neutron.agent.linux.utils:replace_file(). New keyword parameter 'file_mode=0o644' is added to neutron.common.utils:replace_file(). Partial-bug: #1504477 Change-Id: Id1a7f1236786e8606c91bb9925cd9ac8e95892b3 --- neutron/agent/linux/dhcp.py | 24 ++++----- neutron/agent/linux/dibbler.py | 5 +- neutron/agent/linux/keepalived.py | 3 +- neutron/agent/linux/ra.py | 3 +- neutron/common/utils.py | 4 +- neutron/tests/functional/common/__init__.py | 0 neutron/tests/functional/common/test_utils.py | 51 ++++++++++++++++++ neutron/tests/unit/agent/l3/test_agent.py | 2 +- .../unit/agent/l3/test_dvr_local_router.py | 2 +- neutron/tests/unit/agent/linux/test_dhcp.py | 52 +++++++++---------- 10 files changed, 97 insertions(+), 49 deletions(-) create mode 100644 neutron/tests/functional/common/__init__.py create mode 100644 neutron/tests/functional/common/test_utils.py diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 279d2ca7bed..bcf8ae53736 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -26,15 +26,14 @@ from oslo_log import log as logging from oslo_utils import uuidutils import six -from neutron.agent.common import utils as common_utils +from neutron.agent.common import utils as agent_common_utils from neutron.agent.linux import external_process from neutron.agent.linux import ip_lib from neutron.agent.linux import iptables_manager -from neutron.agent.linux import utils from neutron.common import constants from neutron.common import exceptions from neutron.common import ipv6_utils -from neutron.common import utils as commonutils +from neutron.common import utils as common_utils from neutron.extensions import extra_dhcp_opt as edo_ext from neutron.i18n import _LI, _LW, _LE @@ -174,7 +173,7 @@ class DhcpLocalProcess(DhcpBase): version, plugin) self.confs_dir = self.get_confs_dir(conf) self.network_conf_dir = os.path.join(self.confs_dir, network.id) - commonutils.ensure_dir(self.network_conf_dir) + common_utils.ensure_dir(self.network_conf_dir) @staticmethod def get_confs_dir(conf): @@ -199,7 +198,7 @@ class DhcpLocalProcess(DhcpBase): if self.active: self.restart() elif self._enable_dhcp(): - commonutils.ensure_dir(self.network_conf_dir) + common_utils.ensure_dir(self.network_conf_dir) interface_name = self.device_manager.setup(self.network) self.interface_name = interface_name self.spawn_process() @@ -260,7 +259,7 @@ class DhcpLocalProcess(DhcpBase): @interface_name.setter def interface_name(self, value): interface_file_path = self.get_conf_file_name('interface') - utils.replace_file(interface_file_path, value) + common_utils.replace_file(interface_file_path, value) @property def active(self): @@ -596,7 +595,7 @@ class Dnsmasq(DhcpLocalProcess): buf.write('%s %s %s * *\n' % (timestamp, port.mac_address, ip_address)) contents = buf.getvalue() - utils.replace_file(filename, contents) + common_utils.replace_file(filename, contents) LOG.debug('Done building initial lease file %s with contents:\n%s', filename, contents) return filename @@ -666,7 +665,7 @@ class Dnsmasq(DhcpLocalProcess): buf.write('%s,%s,%s\n' % (port.mac_address, name, ip_address)) - utils.replace_file(filename, buf.getvalue()) + common_utils.replace_file(filename, buf.getvalue()) LOG.debug('Done building host file %s with contents:\n%s', filename, buf.getvalue()) return filename @@ -737,7 +736,7 @@ class Dnsmasq(DhcpLocalProcess): if alloc: buf.write('%s\t%s %s\n' % (alloc.ip_address, fqdn, hostname)) addn_hosts = self.get_conf_file_name('addn_hosts') - utils.replace_file(addn_hosts, buf.getvalue()) + common_utils.replace_file(addn_hosts, buf.getvalue()) return addn_hosts def _output_opts_file(self): @@ -746,7 +745,7 @@ class Dnsmasq(DhcpLocalProcess): options += self._generate_opts_per_port(subnet_index_map) name = self.get_conf_file_name('opts') - utils.replace_file(name, '\n'.join(options)) + common_utils.replace_file(name, '\n'.join(options)) return name def _generate_opts_per_subnet(self): @@ -979,7 +978,7 @@ class DeviceManager(object): def __init__(self, conf, plugin): self.conf = conf self.plugin = plugin - self.driver = common_utils.load_interface_driver(conf) + self.driver = agent_common_utils.load_interface_driver(conf) def get_interface_name(self, network, port): """Return interface(device) name for use by the DHCP process.""" @@ -989,7 +988,8 @@ class DeviceManager(object): """Return a unique DHCP device ID for this host on the network.""" # There could be more than one dhcp server per network, so create # a device id that combines host and network ids - return commonutils.get_dhcp_agent_device_id(network.id, self.conf.host) + return common_utils.get_dhcp_agent_device_id(network.id, + self.conf.host) def _set_default_route(self, network, device_name): """Sets the default gateway for this dhcp namespace. diff --git a/neutron/agent/linux/dibbler.py b/neutron/agent/linux/dibbler.py index 3a97f620ef1..f68c45c2242 100644 --- a/neutron/agent/linux/dibbler.py +++ b/neutron/agent/linux/dibbler.py @@ -24,6 +24,7 @@ from neutron.agent.linux import pd from neutron.agent.linux import pd_driver from neutron.agent.linux import utils from neutron.common import constants +from neutron.common import utils as common_utils from oslo_log import log as logging @@ -83,7 +84,7 @@ class PDDibbler(pd_driver.PDDriverBase): buf.write('%s' % SCRIPT_TEMPLATE.render( prefix_path=self.prefix_path, l3_agent_pid=os.getpid())) - utils.replace_file(script_path, buf.getvalue()) + common_utils.replace_file(script_path, buf.getvalue()) os.chmod(script_path, 0o744) dibbler_conf = utils.get_conf_file_name(dcwa, 'client', 'conf', False) @@ -95,7 +96,7 @@ class PDDibbler(pd_driver.PDDriverBase): interface_name='"%s"' % ex_gw_ifname, bind_address='%s' % lla)) - utils.replace_file(dibbler_conf, buf.getvalue()) + common_utils.replace_file(dibbler_conf, buf.getvalue()) return dcwa def _spawn_dibbler(self, pmon, router_ns, dibbler_conf): diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py index 060928e6f4e..09d437e8da3 100644 --- a/neutron/agent/linux/keepalived.py +++ b/neutron/agent/linux/keepalived.py @@ -21,7 +21,6 @@ from oslo_config import cfg from oslo_log import log as logging from neutron.agent.linux import external_process -from neutron.agent.linux import utils from neutron.common import exceptions from neutron.common import utils as common_utils @@ -378,7 +377,7 @@ class KeepalivedManager(object): def _output_config_file(self): config_str = self.config.get_config_str() config_path = self.get_full_config_file_path('keepalived.conf') - utils.replace_file(config_path, config_str) + common_utils.replace_file(config_path, config_str) return config_path diff --git a/neutron/agent/linux/ra.py b/neutron/agent/linux/ra.py index d9eca8d6475..a6fffd32825 100644 --- a/neutron/agent/linux/ra.py +++ b/neutron/agent/linux/ra.py @@ -22,6 +22,7 @@ import six from neutron.agent.linux import external_process from neutron.agent.linux import utils from neutron.common import constants +from neutron.common import utils as common_utils RADVD_SERVICE_NAME = 'radvd' @@ -94,7 +95,7 @@ class DaemonMonitor(object): prefixes=auto_config_prefixes, constants=constants)) - utils.replace_file(radvd_conf, buf.getvalue()) + common_utils.replace_file(radvd_conf, buf.getvalue()) return radvd_conf def _get_radvd_process_manager(self, callback=None): diff --git a/neutron/common/utils.py b/neutron/common/utils.py index 1fe6ab1ce5c..190d3e6242c 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -471,7 +471,7 @@ def round_val(val): rounding=decimal.ROUND_HALF_UP)) -def replace_file(file_name, data): +def replace_file(file_name, data, file_mode=0o644): """Replaces the contents of file_name with data in a safe manner. First write to a temp file and then rename. Since POSIX renames are @@ -485,5 +485,5 @@ def replace_file(file_name, data): dir=base_dir, delete=False) as tmp_file: tmp_file.write(data) - os.chmod(tmp_file.name, 0o644) + os.chmod(tmp_file.name, file_mode) os.rename(tmp_file.name, file_name) diff --git a/neutron/tests/functional/common/__init__.py b/neutron/tests/functional/common/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/neutron/tests/functional/common/test_utils.py b/neutron/tests/functional/common/test_utils.py new file mode 100644 index 00000000000..8515963ae83 --- /dev/null +++ b/neutron/tests/functional/common/test_utils.py @@ -0,0 +1,51 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import os.path +import stat + +from neutron.common import utils +from neutron.tests import base + + +class TestReplaceFile(base.BaseTestCase): + def setUp(self): + super(TestReplaceFile, self).setUp() + temp_dir = self.get_default_temp_dir().path + self.file_name = os.path.join(temp_dir, "new_file") + self.data = "data to copy" + + def _verify_result(self, file_mode): + self.assertTrue(os.path.exists(self.file_name)) + with open(self.file_name) as f: + content = f.read() + self.assertEqual(self.data, content) + mode = os.stat(self.file_name).st_mode + self.assertEqual(file_mode, stat.S_IMODE(mode)) + + def test_replace_file_default_mode(self): + file_mode = 0o644 + utils.replace_file(self.file_name, self.data) + self._verify_result(file_mode) + + def test_replace_file_custom_mode(self): + file_mode = 0o722 + utils.replace_file(self.file_name, self.data, file_mode) + self._verify_result(file_mode) + + def test_replace_file_custom_mode_twice(self): + file_mode = 0o722 + utils.replace_file(self.file_name, self.data, file_mode) + self.data = "new data to copy" + file_mode = 0o777 + utils.replace_file(self.file_name, self.data, file_mode) + self._verify_result(file_mode) diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index a40a29dcc93..ae30cdf9634 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -93,7 +93,7 @@ class BasicRouterOperationsFramework(base.BaseTestCase): self.utils_exec = self.utils_exec_p.start() self.utils_replace_file_p = mock.patch( - 'neutron.agent.linux.utils.replace_file') + 'neutron.common.utils.replace_file') self.utils_replace_file = self.utils_replace_file_p.start() self.external_process_p = mock.patch( diff --git a/neutron/tests/unit/agent/l3/test_dvr_local_router.py b/neutron/tests/unit/agent/l3/test_dvr_local_router.py index 048beb305e7..9609ae8d744 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_local_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_local_router.py @@ -75,7 +75,7 @@ class TestDvrRouterOperations(base.BaseTestCase): self.utils_exec = self.utils_exec_p.start() self.utils_replace_file_p = mock.patch( - 'neutron.agent.linux.utils.replace_file') + 'neutron.common.utils.replace_file') self.utils_replace_file = self.utils_replace_file_p.start() self.external_process_p = mock.patch( diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index 1a25247837c..1604fe7fb70 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/neutron/tests/unit/agent/linux/test_dhcp.py @@ -814,7 +814,7 @@ class TestBase(TestConfBase): self.config_parse(self.conf) self.conf.set_override('state_path', '') - self.replace_p = mock.patch('neutron.agent.linux.utils.replace_file') + self.replace_p = mock.patch('neutron.common.utils.replace_file') self.execute_p = mock.patch('neutron.agent.common.utils.execute') self.safe = self.replace_p.start() self.execute = self.execute_p.start() @@ -993,7 +993,7 @@ class TestDhcpLocalProcess(TestBase): self.assertEqual(lp.interface_name, 'tap0') def test_set_interface_name(self): - with mock.patch('neutron.agent.linux.utils.replace_file') as replace: + with mock.patch('neutron.common.utils.replace_file') as replace: lp = LocalChild(self.conf, FakeDualNetwork()) with mock.patch.object(lp, 'get_conf_file_name') as conf_file: conf_file.return_value = '/interface' @@ -1965,22 +1965,14 @@ class TestDnsmasq(TestBase): class TestDeviceManager(TestConfBase): - - @mock.patch('neutron.agent.linux.dhcp.ip_lib') - @mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver') - def test_setup(self, load_interface_driver, ip_lib): - """Test new and existing cases of DeviceManager's DHCP port setup - logic. - """ - self._test_setup(load_interface_driver, ip_lib, False) - - @mock.patch('neutron.agent.linux.dhcp.ip_lib') - @mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver') - def test_setup_gateway_ips(self, load_interface_driver, ip_lib): - """Test new and existing cases of DeviceManager's DHCP port setup - logic. - """ - self._test_setup(load_interface_driver, ip_lib, True) + def setUp(self): + super(TestDeviceManager, self).setUp() + ip_lib_patcher = mock.patch('neutron.agent.linux.dhcp.ip_lib') + load_interface_driver_patcher = mock.patch( + 'neutron.agent.linux.dhcp.agent_common_utils.' + 'load_interface_driver') + self.mock_ip_lib = ip_lib_patcher.start() + self.mock_load_interface_driver = load_interface_driver_patcher.start() def _test_setup(self, load_interface_driver, ip_lib, use_gateway_ips): # Create DeviceManager. @@ -2044,9 +2036,15 @@ class TestDeviceManager(TestConfBase): 'unique-IP-address/64'])) self.assertFalse(plugin.create_dhcp_port.called) - @mock.patch('neutron.agent.linux.dhcp.ip_lib') - @mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver') - def test_setup_reserved(self, load_interface_driver, ip_lib): + def test_setup_device_manager_dhcp_port_without_gateway_ips(self): + self._test_setup(self.mock_load_interface_driver, + self.mock_ip_lib, use_gateway_ips=False) + + def test_setup_device_manager_dhcp_port_with_gateway_ips(self): + self._test_setup(self.mock_load_interface_driver, + self.mock_ip_lib, use_gateway_ips=True) + + def test_setup_reserved(self): """Test reserved port case of DeviceManager's DHCP port setup logic. """ @@ -2056,7 +2054,7 @@ class TestDeviceManager(TestConfBase): default=False)) plugin = mock.Mock() mgr = dhcp.DeviceManager(self.conf, plugin) - load_interface_driver.assert_called_with(self.conf) + self.mock_load_interface_driver.assert_called_with(self.conf) # Setup with a reserved DHCP port. network = FakeDualNetworkReserved() @@ -2072,7 +2070,7 @@ class TestDeviceManager(TestConfBase): plugin.update_dhcp_port.side_effect = mock_update mgr.driver.get_device_name.return_value = 'ns-XXX' mgr.driver.use_gateway_ips = False - ip_lib.ensure_device_is_ready.return_value = True + self.mock_ip_lib.ensure_device_is_ready.return_value = True mgr.setup(network) plugin.update_dhcp_port.assert_called_with(reserved_port.id, mock.ANY) @@ -2080,9 +2078,7 @@ class TestDeviceManager(TestConfBase): ['192.168.0.6/24'], namespace='qdhcp-ns') - @mock.patch('neutron.agent.linux.dhcp.ip_lib') - @mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver') - def test_setup_reserved_2(self, load_interface_driver, ip_lib): + def test_setup_reserved_2(self): """Test scenario where a network has two reserved ports, and update_dhcp_port fails for the first of those. """ @@ -2092,7 +2088,7 @@ class TestDeviceManager(TestConfBase): default=False)) plugin = mock.Mock() mgr = dhcp.DeviceManager(self.conf, plugin) - load_interface_driver.assert_called_with(self.conf) + self.mock_load_interface_driver.assert_called_with(self.conf) # Setup with a reserved DHCP port. network = FakeDualNetworkReserved2() @@ -2112,7 +2108,7 @@ class TestDeviceManager(TestConfBase): plugin.update_dhcp_port.side_effect = mock_update mgr.driver.get_device_name.return_value = 'ns-XXX' mgr.driver.use_gateway_ips = False - ip_lib.ensure_device_is_ready.return_value = True + self.mock_ip_lib.ensure_device_is_ready.return_value = True mgr.setup(network) plugin.update_dhcp_port.assert_called_with(reserved_port_2.id, mock.ANY)